-
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
Create ActivityHandlerBase, rewrite ActivityHandler for increased extensibility points #1223
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stevengum
requested review from
benbrown,
sgellock,
cwhitten,
cleemullins and
johnataylor
September 27, 2019 18:49
Pull Request Test Coverage Report for Build 81234
💛 - Coveralls |
johnataylor
approved these changes
Sep 27, 2019
* @param context TurnContext A TurnContext representing an incoming Activity from an Adapter | ||
* @param next () => Promise<void> | ||
*/ | ||
protected async handleMessageActivity(context: TurnContext, next: () => Promise<void>): Promise<void> { |
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.
this is now very close to the C# onMessageActivity ... perhaps in the next iteration we could unify further
(FWIW the introduction of functions here is massively better factoring - much clear to read)
stevengum
changed the title
Rewrite ActivityHandler for increased extensibility points
Create ActivityHandlerBase, rewrite ActivityHandler for increased extensibility points
Sep 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds the ActivityHandlerBase with new
on*Activity()
methods. These methods mirror the corresponding C# activities.It also refactors the core logic in
ActivityHandler
andActivityHandler.run()
out into separate overrideable methods. This increases extensibility as developers can change the processing logic and reuse the base logic.The original eventing logic is still included in
ActivityHandler
.None of the already public APIs in
ActivityHandler
have been modified, andActivityHandler.run()
is still the only point of ingesting activities.Specific Changes
ActivityHandlerBase
class used for subclassing. (Closely follows the C# impl ofActivityHandler
)ActivityHandlerBase
ActivityHandler
extendsActivityHandlerBase
and overrides the inheritedon*Activity()
methods fromActivityHandlerBase
.ActivityHandler
now hasprotected
methods that dispatch specific activity types by additional properties on the incoming Activity.Testing
Current ActivityHandler unit tests still work, will add new unit tests (and maybe a TypeScript test collection 🚀)
Todo:
@Kaiqb as FYI