Skip to content

Commit

Permalink
Fix turnContext updateActivity signature
Browse files Browse the repository at this point in the history
Fixes #2937
  • Loading branch information
Josh Gummersall committed Oct 27, 2020
1 parent 820337b commit 31b96ec
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 61 deletions.
2 changes: 1 addition & 1 deletion libraries/botbuilder-core/src/botAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export abstract class BotAdapter {
* @remarks
* Not all channels support this operation. For channels that don't, this call may throw an exception.
*/
public abstract updateActivity(context: TurnContext, activity: Partial<Activity>): Promise<void>;
public abstract updateActivity(context: TurnContext, activity: Partial<Activity>): Promise<ResourceResponse | void>;

/**
* Asynchronously deletes an existing activity.
Expand Down
22 changes: 13 additions & 9 deletions libraries/botbuilder-core/src/testAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,17 @@ export class TestAdapter extends BotAdapter implements ExtendedUserTokenProvider
* Replaces an existing activity in the activeQueue.
* @param context Context object for the current turn of conversation with the user.
* @param activity Activity being updated.
* @returns promise representing async operation
*/
public updateActivity(context: TurnContext, activity: Partial<Activity>): Promise<void> {
for (let i = 0; i < this.activeQueue.length; i++) {
if (activity.id && activity.id === this.activeQueue[i].id) {
this.activeQueue[i] = activity;
break;
public updateActivity(context: TurnContext, activity: Partial<Activity>): Promise<ResourceResponse | void> {
if (activity.id) {
const idx = this.activeQueue.findIndex((a) => a.id === activity.id);
if (idx !== -1) {
this.activeQueue.splice(idx, 1, activity);
}
return Promise.resolve({ id: activity.id });
}

return Promise.resolve();
}

Expand All @@ -295,12 +298,13 @@ export class TestAdapter extends BotAdapter implements ExtendedUserTokenProvider
* @param reference `ConversationReference` for activity being deleted.
*/
public deleteActivity(context: TurnContext, reference: Partial<ConversationReference>): Promise<void> {
for (let i = 0; i < this.activeQueue.length; i++) {
if (reference.activityId && reference.activityId === this.activeQueue[i].id) {
this.activeQueue.splice(i, 1);
break;
if (reference.activityId) {
const idx = this.activeQueue.findIndex((a) => a.id === reference.activityId);
if (idx !== -1) {
this.activeQueue.splice(idx, 1);
}
}

return Promise.resolve();
}

Expand Down
75 changes: 35 additions & 40 deletions libraries/botbuilder-core/src/turnContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,45 +483,49 @@ export class TurnContext {
*/
public sendActivities(activities: Partial<Activity>[]): Promise<ResourceResponse[]> {
let sentNonTraceActivity = false;
const ref: Partial<ConversationReference> = TurnContext.getConversationReference(this.activity);
const output: Partial<Activity>[] = activities.map((a: Partial<Activity>) => {
const o: Partial<Activity> = TurnContext.applyConversationReference({ ...a }, ref);
if (!o.type) {
o.type = ActivityTypes.Message;
const ref = TurnContext.getConversationReference(this.activity);
const output = activities.map((activity) => {
const result = TurnContext.applyConversationReference({ ...activity }, ref);

if (!result.type) {
result.type = ActivityTypes.Message;
}
if (o.type !== ActivityTypes.Trace) {

if (result.type !== ActivityTypes.Trace) {
sentNonTraceActivity = true;
}
if (o.id) {
delete o.id;

if (result.id) {
delete result.id;
}
return o;

return result;
});

return this.emit(this._onSendActivities, output, () => {
return this.emit(this._onSendActivities, output, async () => {
if (this.activity.deliveryMode === DeliveryModes.ExpectReplies) {
// Append activities to buffer
const responses: ResourceResponse[] = [];
output.forEach((a) => {
this.bufferedReplyActivities.push(a);
responses.push({ id: undefined });
this.bufferedReplyActivities.push(a);
responses.push({ id: undefined });
});

// Set responded flag
if (sentNonTraceActivity) {
this.responded = true;
}

return Promise.resolve(responses);
return responses;
} else {
return this.adapter.sendActivities(this, output).then((responses: ResourceResponse[]) => {
// Set responded flag
if (sentNonTraceActivity) {
this.responded = true;
}
const responses = await this.adapter.sendActivities(this, output);

return responses;
});
// Set responded flag
if (sentNonTraceActivity) {
this.responded = true;
}

return responses;
}
});
}
Expand Down Expand Up @@ -550,7 +554,7 @@ export class TurnContext {
* - [deleteActivity](xref:botbuilder-core.TurnContext.deleteActivity)
* - [getReplyConversationReference](xref:botbuilder-core.TurnContext.getReplyConversationReference)
*/
public updateActivity(activity: Partial<Activity>): Promise<void> {
public updateActivity(activity: Partial<Activity>): Promise<ResourceResponse | void> {
const ref: Partial<ConversationReference> = TurnContext.getConversationReference(this.activity);
const a: Partial<Activity> = TurnContext.applyConversationReference(activity, ref);
return this.emit(this._onUpdateActivity, a, () => this.adapter.updateActivity(this, a));
Expand Down Expand Up @@ -804,26 +808,17 @@ export class TurnContext {

/**
* @private
* Executes `handlers` as a chain, returning a promise that resolves to the final result.
*/
private emit<T>(
handlers: ((context: TurnContext, arg: T, next: () => Promise<any>) => Promise<any>)[],
arg: T,
next: () => Promise<any>
): Promise<any> {
const list: ((context: TurnContext, arg: T, next: () => Promise<any>) => Promise<any>)[] = handlers.slice();
const context: TurnContext = this;
function emitNext(i: number): Promise<void> {
try {
if (i < list.length) {
return Promise.resolve(list[i](context, arg, () => emitNext(i + 1)));
}

return Promise.resolve(next());
} catch (err) {
return Promise.reject(err);
}
}
private emit<A, T>(
handlers: Array<(context: TurnContext, arg: A, next: () => Promise<T>) => Promise<T>>,
arg: A,
next: () => Promise<T>
): Promise<T> {
const runHandlers = ([handler, ...remaining]: typeof handlers): Promise<T> => {
return handler ? handler(this, arg, () => runHandlers(remaining)) : next();
};

return emitNext(0);
return runHandlers(handlers);
}
}
4 changes: 2 additions & 2 deletions libraries/botbuilder-dialogs/tests/numberPrompt.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ describe('NumberPrompt', function () {
(dialogContext) =>
dialogContext.prompt('prompt', { prompt: 'Send me a zero', retryPrompt: 'Send 0 or zero' }),
(turnContext) => turnContext.sendActivity('ok'),
(prompt) => {
async (prompt) => {
if (prompt.recognized.value !== 0) {
prompt.context.sendActivity(`attemptCount ${prompt.attemptCount}`);
await prompt.context.sendActivity(`attemptCount ${prompt.attemptCount}`);
return false;
}

Expand Down
7 changes: 4 additions & 3 deletions libraries/botbuilder/src/botFrameworkAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1377,7 +1377,7 @@ export class BotFrameworkAdapter
* @remarks
* Not all channels support this operation. For channels that don't, this call may throw an exception.
*/
public async updateActivity(context: TurnContext, activity: Partial<Activity>): Promise<void> {
public updateActivity(context: TurnContext, activity: Partial<Activity>): Promise<ResourceResponse | void> {
if (!activity.serviceUrl) {
throw new Error(`BotFrameworkAdapter.updateActivity(): missing serviceUrl`);
}
Expand All @@ -1387,8 +1387,9 @@ export class BotFrameworkAdapter
if (!activity.id) {
throw new Error(`BotFrameworkAdapter.updateActivity(): missing activity.id`);
}
const client: ConnectorClient = this.getOrCreateConnectorClient(context, activity.serviceUrl, this.credentials);
await client.conversations.updateActivity(activity.conversation.id, activity.id, activity as Activity);

const client = this.getOrCreateConnectorClient(context, activity.serviceUrl, this.credentials);
return client.conversations.updateActivity(activity.conversation.id, activity.id, activity as Activity);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions libraries/botbuilder/src/skills/skillHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,8 @@ export class SkillHandler extends ChannelServiceHandler {
activityId: string,
activity: Activity
): Promise<ResourceResponse> {
let resourceResponse: ResourceResponse | void;

await this.continueConversation(claimsIdentity, conversationId, async (adapter, ref, context) => {
const newActivity = TurnContext.applyConversationReference(activity, ref.conversationReference);

Expand All @@ -323,13 +325,11 @@ export class SkillHandler extends ChannelServiceHandler {
claimsIdentity.claims
)}`;

return context.updateActivity(newActivity);
resourceResponse = await context.updateActivity(newActivity);
});

// Note: the original activity ID is passed back here to provide "behavioral" parity with the C# SDK. Due to
// some inconsistent method signatures, the proper response is not propagated back through `context.updateActivity`
// so we have to manually pass this value back.
return { id: activityId };
// Due to a backwards-compat function signature, resourceResponse may not be defined.
return resourceResponse ? resourceResponse : { id: activityId };
}

/**
Expand Down
3 changes: 2 additions & 1 deletion libraries/botbuilder/tests/botFrameworkAdapter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,8 @@ describe(`BotFrameworkAdapter`, function () {
it(`should updateActivity().`, async () => {
const adapter = new AdapterUnderTest();
const context = new TurnContext(adapter, incomingMessage);
await adapter.updateActivity(context, incomingMessage);
const result = await adapter.updateActivity(context, incomingMessage);
assert.deepStrictEqual(result, { id: '5678' }, 'result is expected');
});

it(`should fail to updateActivity() if serviceUrl missing.`, function (done) {
Expand Down

0 comments on commit 31b96ec

Please sign in to comment.