Botbuilder-js: [BotBuilder] TeamsActivityHandler class does not match C# counterpart

[Batta32]

Versions

What package version of the SDK are you using.
[email protected]
What nodejs version are you using
12.16.1
What browser version are you using
-
What os are you using
Windows 10 Pro

Describe the bug

Give a clear and concise description of what the bug is.
The JavaScript implementation of TeamsActivityHandler contains different method's signatures compared to C#.

The methods in JavaScript uses the prefix handle while C# uses on.

To Reproduce

Steps to reproduce the behavior:

1. Clone BotBuilder-DotNet
2. Navigate to /libraries/Microsoft.Bot.Builder/Teams/
3. Open TeamsActivityHandler.cs
4. Scroll down to line 116
5. Clone BotBuilder-JS
6. Navigate to /libraries/botbuilder/src/
7. Open teamsActivityHandler.ts
8. Scroll down to line 221
9. See the differences

More differences are present between the files.

Expected behavior

Give a clear and concise description of what you expected to happen.
I expect TeamsActivityHandler in JavaScript to use the same implementation as in C#.

Screenshots

If applicable, add screenshots to help explain your problem.
_Disparity in the onSignInInvoke method between C# and JavaScript_

_handleTeamsSignInVerifyState vs OnTeamsSignInVerifyStateAsync_

_handleTeamsO365ConnectorCardAction vs OnTeamsO365ConnectorCardActionAsync_

Additional context

Add any other context about the problem here.
C# Virtual Assistant overrides the OnTeamsSignInVerifyStateAsync method while TypeScript Virtual Assistant overrides the handleTeamsSignInVerifyState method.

[stevengum]

Thanks for filing this @Batta32! This is by design, the initial ActivityHandler implementations in .NET and TypeScript differed to try and address a lack of idiomaticness perceived in TypeScript. This difference manifested in the event listener-derived TypeScript implementation shipped in 4.3:

https://github.com/microsoft/botbuilder-js/blob/c9b4001112d7dc428fcbb8f3629b481af73a8cec/libraries/botbuilder-core/src/activityHandler.ts#L65-L66

Whereas the .NET implementation followed a classic inheritance pattern through public virtual methods.

public virtual Task OnTurnAsync(ITurnContext turnContext, CancellationToken cancellationToken = default(CancellationToken))

In botbuilder-js, the ActivityHandlerBase class added in 4.6.0 matches the implementation of the 4.3.0 .NET ActivityHandler. This was added to better support Invoke activity handling for Teams. For Invoke Activities the bot must respond with a single InvokeResponse. This is normally returned by the buisness logic in the bot.

To avoid breaking changes, the method names are changed in TypeScript.

For ActivityHandler methods:

• on<ActivityType>Activity represents the inheritance model (first defined in ActivityHandlerBase)
• on<ActivityType> represents the event listener pattern

For TeamsActivityHandler methods:

• onTeams*Event represents the event listener pattern for Teams ConversationUpdates
• onTeams* represents the inheritance model for Teams ConversationUpdates

• handleTeams* is used to handle Teams Invoke Activities

  In .NET it was simple to plumb this through as they were already using an inheritance-based model, but for TypeScript the underlying ActivityHandler class needed to be reimplemented without breaking changes, which is why we have handleTeams*() in TS but OnTeams*() in .NET.

---

To mirror the .NET VA implementation, one path would be subclassing the TeamsActivityHandler and then overriding and consuming the following methods (mentioned above):

• on<ActivityType>Activity represents the inheritance model (first defined in ActivityHandlerBase)
• onTeams* represents the inheritance model for Teams ConversationUpdates
• handleTeams* is used to handle Teams Invoke Activities

[Batta32]

Awesome @stevengum, thank you for the provided information.

We already applied that approach in the microsoft/botframework-solutions#3584 for 1.0 TypeScript Virtual Assistant (see DefaultActivityHandler).

If this is by design, feel free to close this issue 😊.

[stevengum]

Great! Closing issue as the implementation differences are by design.