Skip to content
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

Pattern for accepting more activity types #65

Open
bmorton opened this issue Dec 18, 2021 · 1 comment
Open

Pattern for accepting more activity types #65

bmorton opened this issue Dec 18, 2021 · 1 comment

Comments

@bmorton
Copy link
Contributor

bmorton commented Dec 18, 2021

Hey! I'm looking at supporting the other activity types that could arise, but it looks like changes to the core library are required to make that possible.

Would you be interested in a PR for either of these solutions?

  • A fallback handler so that if any other activity isn't handled, the fallback handler is invoked and is given the opportunity to handle unsupported activities. This should be backwards compatible, but isn't as clean.
  • Or, turn handlers into a map[schema.ActivityType]func(turn *activity.TurnContext) (schema.Activity, error) and allow for handlers for any ActivityType to be defined that way. This way seems potentially cleaner to me, but also looks like a breaking change.
  • Or, continue out the pattern as it is in the repository with the rest of the ActivityTypes. This would be backwards compatible, but would have a bit of duplication.
@bmorton
Copy link
Contributor Author

bmorton commented Jan 4, 2022

Hey @PrasadG193 -- do you have opinions on which of these approaches would work? The first is probably the cleanest, but would then be a breaking change when activities move from "unsupported" to "supported."

Ultimately, I'd like to do the second option because it allows us to support all the different types that may arise and also allows a fallback handler to be defined that doesn't suffer from the above breaking change since the developer always has the option to implement any schema.ActivityType instead of letting the fallback handle it.

To do this, we'd have to change the signature of (*BotFrameworkAdapter).ProcessActivity since we'd have to pass a different type of handler. Spiking out some code, I think usage would look like this:

customHandler := core.NewActivityHandler()
customHandler.Register(schema.Message, func(turn *activity.TurnContext) (schema.Activity, error) {
	return turn.SendActivity(activity.MsgOptionText("Echo: " + turn.Activity.Text))
});

Then we could either include it with the adapter or with the current pattern (both of these would be a breaking change):

// Current pattern
err = ht.Adapter.ProcessActivity(ctx, activity, customHandler)

// At bot adapter creation
adapter, err := core.NewBotAdapter(core.AdapterSetting{
	AppID:       os.Getenv("APP_ID"),
	AppPassword: os.Getenv("APP_PASSWORD"),
	ActivityHandler: customHandler,
})

Thoughts on this approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant