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.

Sign up for GitHub

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

Jump to bottom

[Core] Add CustomWebAdapter (+OAuth) #149

Merged
merged 39 commits into from
Mar 20, 2020
Merged

[Core] Add CustomWebAdapter (+OAuth) #149

merged 39 commits into from
Mar 20, 2020

Conversation

iMicknl
Copy link
Contributor

@iMicknl iMicknl commented Feb 6, 2020

Based on #141 & #148.

@iMicknl iMicknl mentioned this pull request Feb 6, 2020
Closed
@nestoralonsovina
Copy link

I've tested the customWebAdapter to build another WhatsApp adapter for other provider, and everything seems to work correctly. (OAuth included)

Having everything there reduces quite a lot the time needed to build an adapter.

@iMicknl
Copy link
Contributor Author

iMicknl commented Feb 21, 2020

@nestoralonsovina thanks for testing! For which provider are you building a WhatsApp adapter? Would be cool to have it added to the repo, if possible. ;-)

The only thing left for this PR is to make sure that there is a good fallback when the function is called, when there are no credentials.

Currently there are some changes coming to the OAuth implementation in BotFramework adapter, I would like to wait for those changes until merging this one in.

@nestoralonsovina
Copy link

@iMicknl is a private local provider so I don't think there is any use for it.

I will take a look at the incoming changes too.

@iMicknl iMicknl mentioned this pull request Feb 28, 2020
Merged
@iMicknl
Copy link
Contributor Author

iMicknl commented Mar 11, 2020 •
edited
Loading

@nestoralonsovina could you possibly share how you are calling the OAuth flow? I couldn't get it working by calling the OAuthPrompt, this PR microsoft/botbuilder-js#1812 solved it for me.

I have updated this PR with the newest OAuth changes and just tested it using the Twilio WhatsApp adapter.

IMG_7024

@iMicknl
Copy link
Contributor Author

iMicknl commented Mar 11, 2020

Currently blocked by 4.8 release of Bot Framework, which should go out soon.

@iMicknl iMicknl requested a review from szul March 11, 2020 21:30
@iMicknl
Copy link
Contributor Author

iMicknl commented Mar 11, 2020

@szul could you possibly give it a quick look? I would love to release this one when 4.8 is released, afterwards I will update the Alexa + Twilio WhatsApp adapter to use this base.

Do you think this one could also be used for the adapters you created? Otherwise, I would love to know what we can do differently.

This was referenced Mar 12, 2020
Closed
Merged
@nestoralonsovina
Copy link

@iMicknl I implemented the same changes as in microsoft/botbuilder-js#1812, so 4.8 would fix it.

Sadly they just hardcoded the adapters in the code, so it might be easier if we just override the isOAuthCardSupported function (once is out) in our CustomWebAdapter, since (if I'm not mistaken) none of the custom adapters (Twitter, Twilio, etc) will handle the oAuthCards.

@iMicknl
Copy link
Contributor Author

iMicknl commented Mar 12, 2020 •
edited
Loading

@nestoralonsovina I am also not a huge fan of the hardcoded names, however I fixed it in my custom adapter by setting the name to Web Adapter. source. This doesn't conflict with anything else. The screenshot above is using the OAuthPrompt with the Twilio adapter, which is using the new CustomWebAdapter of this PR.

Thanks for confirming. I will merge and publish this package as soon as 4.8 is released.

iMicknl and others added 10 commits March 15, 2020 14:49
@iMicknl iMicknl marked this pull request as ready for review March 18, 2020 10:07
@iMicknl iMicknl changed the base branch from master to develop March 20, 2020 15:57
@iMicknl
Squashed commit of the following:
03843bf 
commit fa80f07
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Fri Mar 20 16:55:40 2020 +0100

    Merge pull request BotBuilderCommunity#163

    [Core] Update all packages to botbuilder 4.8.0 and correct multiple package-lock.json BotBuilderCommunity#163

commit 097923e
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Fri Mar 20 16:46:03 2020 +0100

    Sync master -> develop (BotBuilderCommunity#164)

    * Adaptive card dialog needed to be added to export

    * Audit fix

    Co-authored-by: Michael Szul <michael@szul.us>

commit 1f1b479
Merge: 7a44067 d2ba4e6
Author: Kyle Delaney <kyled@aditiconsulting.com>
Date:   Fri Feb 14 08:54:46 2020 -0800

    Merge pull request BotBuilderCommunity#157 from iMicknl/adapter/alexa

    [Alexa] Add Alexa Adapter

commit d2ba4e6
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Fri Feb 14 16:03:49 2020 +0100

    Update dependencies

commit 8af037e
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Fri Feb 14 16:02:03 2020 +0100

    Bugfix: don’t break on unsupported activities

commit 2c88331
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Wed Feb 12 00:44:43 2020 +0100

    Update sample to the beta package

commit 216d18b
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Wed Feb 12 00:40:59 2020 +0100

    Update publish config

commit 17791d0
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Wed Feb 12 00:31:40 2020 +0100

    Minor changes in documentation and version number

commit d1eee53
Author: Mick Vleeshouwer <mick@imick.nl>
Date:   Wed Feb 12 00:14:48 2020 +0100

    First preview version of Alexa Adapter

commit 7a44067
Merge: fa478f7 f1a469d
Author: Michael Szul <michael@szul.us>
Date:   Tue Feb 11 07:44:09 2020 -0500

    Merge pull request BotBuilderCommunity#156 from BotBuilderCommunity/master

    Updated .npmignore to remove .nyc_output

commit fa478f7
Merge: b8922d8 dd860af
Author: Michael Szul <michael@szul.us>
Date:   Tue Feb 11 07:35:48 2020 -0500

    Merge pull request BotBuilderCommunity#154 from BotBuilderCommunity/master

    [Twilio-WhatsApp] Add support for location messages (inbound & o… (BotBuilderCommunity#142)
@iMicknl iMicknl merged commit d2b6731 into BotBuilderCommunity:develop Mar 20, 2020
@iMicknl iMicknl deleted the core-work+oauth branch March 20, 2020 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nestoralonsovina nestoralonsovina nestoralonsovina left review comments

@szul szul Awaiting requested review from szul

@stephanbisser stephanbisser Awaiting requested review from stephanbisser stephanbisser is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@iMicknl @nestoralonsovina @szul