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

All CAP features #21

Merged
merged 9 commits into from Apr 15, 2019
Merged

All CAP features #21

merged 9 commits into from Apr 15, 2019

Conversation

robin-liao
Copy link
Contributor

@robin-liao robin-liao commented Apr 4, 2019

  1. Adaptive Card
  2. BotBuilder actions wrapped into Adaptive Card
  3. Task Module
  4. Messaging Extension - create card
  5. Messaging Extension - message actions
  6. Messaging Extension - message preview
  7. Messaging Extension - app-based link
  8. FileBot sample (user ->bot & bot -> user file sending)

Copy link
Member

@RamjotSingh RamjotSingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robin-liao You have added a bunch of features, can you add one sample for each of them. One for TaskModule, one for file upload etc.

using System.Threading.Tasks;
using Microsoft.Bot.Schema.Teams;

/// <summary>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this class even work in most cases? If the invoke responses were sent as null wouldn't BotFramework throw BadRequest?

Copy link
Contributor Author

@robin-liao robin-liao Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the implementation of Task.FromResult<InvokeResponse>(null); from your Wiki example. Of course it won't work for each case since for some cases invoke response requires a specific type. My intention is to have a default implementation so as to prevent from breaking existing classes given adding more invokes in the future. Note that we're having a bunch of invoke handlers and we're adding more for sure. Having one more newly added interface will potential break all existing derived classes as they're directly implementing the set of interface (instead of deriving from base class) and there's no choice to have optional interface implementation by default. To keep a default base class can avoid this design problem. An alternative solution is to throw exceptions for non-implemented methods instead of return null. Thought?

@robin-liao
Copy link
Contributor Author

@robin-liao You have added a bunch of features, can you add one sample for each of them. One for TaskModule, one for file upload etc.

@RamjotSingh I have added a MessageExtension bot in this PR that includes all the CAP features. It aligns with the Node SDK example. But I missed the file part. I can add one more file bot as I also did it in Node SDK.

@robin-liao
Copy link
Contributor Author

@robin-liao You have added a bunch of features, can you add one sample for each of them. One for TaskModule, one for file upload etc.

@RamjotSingh I have added a MessageExtension bot in this PR that includes all the CAP features. It aligns with the Node SDK example. But I missed the file part. I can add one more file bot as I also did it in Node SDK.

@RamjotSingh added a FileBot sample, which aligns the sample in Node SDK

@robin-liao robin-liao merged commit ddbcd70 into master Apr 15, 2019
@robin-liao robin-liao deleted the yilia/cap-features branch April 15, 2019 21:14
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

Successfully merging this pull request may close these issues.

None yet

3 participants