Add locale support in DirectLine options#293
Conversation
timenick
changed the title
timenick
changed the title
|
@timenick I’ll look into this tomorrow, but we likely need changes in the Emulator as well |
|
After talking to @timenick, because the service change is not on production yet, thus, he cannot add any tests yet. He will test it manually, then add tests in another PR. |
| const body = this.conversationId | ||
| ? undefined | ||
| : { | ||
| locale: this.localeOnStartConversation |
There was a problem hiding this comment.
if conversationStartProperties is not sent in or does not contain a locale, should we still send a body? I would assume not since we dont want to change any existing client.
There was a problem hiding this comment.
In our backend, we are reading from body to get the conversationStartParameters. Because we are not only sending locale in body, we would send more parameters (like userid, username, etc) in the future. The locale parameter is optional, it would not affect any existing client.
| timeout: this.timeout, | ||
| headers: { | ||
| "Accept": "application/json", | ||
| "Content-Type": "application/json", |
There was a problem hiding this comment.
Add a test to make sure adding a Content-Type doesnt affect existing clients that dont send a body.
p-nagpal
left a comment
p-nagpal
left a comment
There was a problem hiding this comment.
Lets add tests
- DirectLineJS does not receive conversation start parameters (tests that existing clients are not affected)
- DirectLineJS does not receive a locale in the conversation start parameters
- DirectLineJS receives a valid locale in the conversation start parameters
- DirectLineJS receives a invalid locale in the conversation start parameters
Sure. I will update the test bot first. And will sync with William for the tests. |
src/directLine.ts
Outdated
| this.token = options.secret || options.token; | ||
| this.webSocket = (options.webSocket === undefined ? true : options.webSocket) && typeof WebSocket !== 'undefined' && WebSocket !== undefined; | ||
|
|
||
| if (options.conversationStartProperties) { |
There was a problem hiding this comment.
| if (options.conversationStartProperties) { | |
| if (options.conversationStartProperties && options.conversationStartProperties.locale) { |
Combine two ifs into one.
4 participants
fixes #292