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

Locale Fixes #2105

Merged
merged 5 commits into from
Apr 27, 2020
Merged

Locale Fixes #2105

merged 5 commits into from
Apr 27, 2020

Conversation

mdrichardson
Copy link
Contributor

@mdrichardson mdrichardson commented Apr 25, 2020

Fixes #2093
Fixes #2032 (most of this was done in this PR, except for one tiny change to SkillHandler.

Description

  • Adds locale for EndOfConversation events and activities
  • Adds locale to ConversationReference

Testing

Added checks for locale in existing tests.

@@ -351,9 +351,11 @@ describe(`BotFrameworkAdapter`, function () {

it('ConnectorClient should use httpClient from clientOptions', async () => {
let sendRequestCalled = false;
const outgoingMessageLocale = JSON.parse(JSON.stringify(outgoingMessage));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to make a change to this test because instead of an activity with no locale coming back as:

{
  ...
  text: 'blah'
  ...
}

...it would come back as:

{
  ...
  text: 'blah',
  locale: undefined
  ...
}

I'm a little worried that activity.locale = reference.locale (in the above changes) could be a rare breaking change (should really only happen with something like assert.deepEqual() or if (activity.locale !== undefined)(?). This could be fixed by using something more like:

if (!reference.locale) { activity.locale = reference.locale; }

...but I figured the very low risk made the readability okay. Let me know if you feel otherwise.

@coveralls
Copy link

coveralls commented Apr 25, 2020

Pull Request Test Coverage Report for Build 125203

  • 5 of 5 (100.0%) changed or added relevant lines in 3 files are covered.
  • 9 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.07%) to 79.707%

Files with Coverage Reduction New Missed Lines %
libraries/botframework-streaming/src/webSocket/webSocketTransport.ts 2 80.65%
libraries/botframework-streaming/src/webSocket/webSocketServer.ts 7 66.67%
Totals Coverage Status
Change from base Build 125201: -0.07%
Covered Lines: 12303
Relevant Lines: 14728

💛 - Coveralls

Copy link
Contributor

@Stevenic Stevenic left a comment

Choose a reason for hiding this comment

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

:shipit:

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] Locale Fixes [PORT] Updated RunAsync to avoid sending EoC from the RootBot to the channel
3 participants