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

Updated OAuthInput to be on parity with dotnet sdk #2349

Merged
merged 6 commits into from
Jul 7, 2020
Merged

Conversation

chon219
Copy link
Member

@chon219 chon219 commented Jun 15, 2020

Fixes #2034
Fixes #2152
Fixes #2396

Description

Updated OAuthInput to be on parity with dotnet sdk.

Specific Changes

  • Updated implementation of OAuthInput
  • Fixed alwaysPrompt related bug in OAuthInput
  • Fixed interruption related bug in OAuthInput

Testing

We don't have unit tests for OAuthInput in both c# and js sdk, we may need to test it e2e to make sure everything work as expected.

@coveralls
Copy link

coveralls commented Jun 15, 2020

Pull Request Test Coverage Report for Build 143845

  • 9 of 110 (8.18%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.4%) to 80.607%

Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/botbuilder-dialogs-adaptive/src/input/oauthInput.ts 9 110 8.18%
Files with Coverage Reduction New Missed Lines %
libraries/botbuilder-dialogs-adaptive/src/input/oauthInput.ts 3 3.95%
Totals Coverage Status
Change from base Build 143813: -0.4%
Covered Lines: 12979
Relevant Lines: 15377

💛 - Coveralls

@chon219
Copy link
Member Author

chon219 commented Jun 23, 2020

We should fix #2152 and #2396 as well in this PR are they are all about oauthinput.

@chon219
Copy link
Member Author

chon219 commented Jun 23, 2020

We may need to test OAuthInput E2E to make sure it will work as expected.

Copy link
Contributor

@luhan2017 luhan2017 left a comment

Choose a reason for hiding this comment

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

I have tested this PR e2e and found that the OAuthInput doesn't work as expected in skill mode. I think we need to port the updates from this commit, 1a2a64a

Copy link
Contributor

@luhan2017 luhan2017 left a comment

Choose a reason for hiding this comment

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

Test e2e for both bot mode and skill mode, both work well.

@chon219 chon219 merged commit 27a5b29 into master Jul 7, 2020
@cleemullins cleemullins deleted the zim/oauth-input branch August 11, 2020 22:26
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.

[PORT] Update OAuthInput.cs [PORT] OAuthInput: create parity with OAuthPrompt [PORT] Fix OAuth bug
3 participants