-
Notifications
You must be signed in to change notification settings - Fork 276
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
fix: remove Cortana from Channels enum #3730
Merged
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
added
Automation: Parity with dotnet
The PR needs to be ported to dotnet.
Automation: Parity with Java
The PR needs to be ported to Java.
Automation: Parity with Python
The PR needs to be ported to Python
labels
Jun 7, 2021
3 tasks
mdrichardson
reviewed
Jun 7, 2021
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.
Edit: Meant to post comment on different PR.
mdrichardson
reviewed
Jun 8, 2021
mdrichardson
approved these changes
Jun 8, 2021
joshgummersall
pushed a commit
that referenced
this pull request
Jun 8, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 9, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 14, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 15, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 16, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 18, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 18, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 21, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 22, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
joshgummersall
pushed a commit
that referenced
this pull request
Jun 22, 2021
* remove Cortana from Channels enum * clean up linted choices_* tests
stevengum
added a commit
that referenced
this pull request
Jun 23, 2021
* feat: add additional BotFrameworkAuthentication classes for CloudAdapter (#3698) * add additional BotFrameworkAuthentication classes for CloudAdapter * Add Alexa to Channels enum for #3603 * fix export, USER_AGENT and style issue * make path in Configuration.get optional, apply other PR feedback * address additional PR feedback, minor cleanup * update adaptive-runtime.Configuration to reflect contract * cleanup typing, add unit tests, refactor existing code * fix linting error in types.ts * fix: remove Cortana from Channels enum (#3730) * remove Cortana from Channels enum * clean up linted choices_* tests * feat: cloud adapter (#3728) * port: cloud adapter base - remove unnecessary emitter type - streaming syntax issues - introduce runtypes helpers - bot framework adapter implements http adapter interface * add created status code, remove dupe status codes Not sure how the duplicate status codes piece was introduced, seems unnecessary though. * add continue conv w/ claims to bot adapter This enables us to properly pass claims to cloud adapter without plumbing them all the way through turn context. This is handled in .NET via overloads, however we can't achieve the same thing due to backwards-compat requirements in BotAdapter. See .NET overloads: https://github.com/microsoft/botbuilder-dotnet/blob/3c335046f95deeac50fbb0b48c7c8c42051d4f6d/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs#L147-L163 * connectory factory create -> audience? * cloud adapter + associated runtime changes - properly handle streaming requests - include retries for named pipes * cloud skill handler changes - better http errors for channel service routes - clean up channel service handler tests This is essentially a port of microsoft/botbuilder-dotnet#5304 * better docs/comments all around * plumb errors through to http responses - req + res in turn context, use in adapter error handler - fix several errors that should have yielded a 401 * port builtin auth handler logic This accounts for the no-auth (Emulator) skills scenario. The logic exists in .NET here: https://github.com/microsoft/botbuilder-dotnet/blob/3c335046f95deeac50fbb0b48c7c8c42051d4f6d/libraries/Microsoft.Bot.Connector/Authentication/BuiltinBotFrameworkAuthentication.cs#L72-L87 * fix anonymous auth handling The default scenario after creating a bot using Composer is a settings file where `MicrosoftAppId` is set to `""`, aka the empty string. When Emulator sends requests without an app ID/password, the auth header is missing and thus produces a null app ID (identity.getClaims returns null). This commit properly accounts for this anonymous auth scenario. * fix runtime constructor exception The default runtime config does not set any of the configuration values related to auth. As a result, runtypes checking fails. This change coerces the runtype into a partial (to match the contructor signature) and assigns the checked value. Also, - fix boolean coercian to stringify value - use import type * small runtypes cleanup Also remove unnecessary "as" casting * remove extra call to isValidAppId This call does not exist in the .NET counterpart: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Connector/Authentication/PasswordServiceClientCredentialFactory.cs#L76 * fix dialog context error message issue * cloud adapter base -> core Also extract response error handling to integrations as base no longer knows about HTTP specifics * user token access helper Also, refactor adaptive oauth input using oauth prompt * reconcile core bot adapter changes * handle streaming version request Note that token handling also does not exist in .NET: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs#L517 * support service client credentials factory in runtime * restore appId validation to pwServiceCredentialFactory.createCredentials() - remove "only-warn" plugin - add tests * quick PR feedback fixes * remove duplicate call to create connector client * continueConversationAsync with overloads * finishing touches - remaining PR items - remove BotLogic type - test:compat fixes Co-authored-by: stevengum <14935595+stevengum@users.noreply.github.com> * fix: remaining runtypes -> zod (#3789) * fix: test:compat * fix: tests Co-authored-by: Steven Gum <14935595+stevengum@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Automation: Parity with dotnet
The PR needs to be ported to dotnet.
Automation: Parity with Java
The PR needs to be ported to Java.
Automation: Parity with Python
The PR needs to be ported to Python
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.
Fixes #3603
Targeting
cloud-adapter
to avoid future merge conflictsSpecific Changes
Channels
enum in botframework-schema, see Add Alexa and remove Cortana from Channels enum #3603 (comment)hasMessageFeed()
always returnstrue
(no change on visibility in declarations)supportsCardActions()
no longer hasChannels.Cortana
case (will use defaultfalse
for'cortana'
channelId).choices_channel.test.js
andchoices_choiceFactory.test.js
OAuthPrompt.channelSupportsOAuthCard()
no longer hasChannels.Cortana
case (will use defaulttrue
for'cortana'
channelId).