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

fix: remove Cortana from Channels enum #3730

Merged
merged 2 commits into from
Jun 8, 2021
Merged

Conversation

stevengum
Copy link
Member

Fixes #3603

Targeting cloud-adapter to avoid future merge conflicts

Specific Changes

  • Remove Cortana from Channels enum in botframework-schema, see Add Alexa and remove Cortana from Channels enum #3603 (comment)
  • Hidden public helper hasMessageFeed() always returns true (no change on visibility in declarations)
  • supportsCardActions() no longer has Channels.Cortana case (will use default false for 'cortana' channelId).
  • remove/update Cortana specific tests in choices_channel.test.js and choices_choiceFactory.test.js
  • OAuthPrompt.channelSupportsOAuthCard() no longer has Channels.Cortana case (will use default true for 'cortana' channelId).

@stevengum 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
@stevengum stevengum requested review from a team as code owners June 7, 2021 23:38
@stevengum stevengum requested review from Stevenic and removed request for a team June 7, 2021 23:38
Copy link
Contributor

@mdrichardson mdrichardson left a 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.

@stevengum stevengum merged commit 081c448 into cloud-adapter Jun 8, 2021
@stevengum stevengum deleted the stgum/removeCortana branch June 8, 2021 00:37
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants