From 02eeb9a23ed3e5483effe9140f46b4f832d56218 Mon Sep 17 00:00:00 2001 From: Steven Gum <14935595+stevengum@users.noreply.github.com> Date: Mon, 7 Jun 2021 17:37:06 -0700 Subject: [PATCH] fix: remove Cortana from Channels enum (#3730) * remove Cortana from Channels enum * clean up linted choices_* tests --- .../botbuilder-dialogs/src/choices/channel.ts | 18 +- .../src/prompts/oauthPrompt.ts | 1 - .../tests/choices_channel.test.js | 99 ++++----- .../tests/choices_choiceFactory.test.js | 191 +++++++++--------- libraries/botframework-schema/src/index.ts | 1 - 5 files changed, 137 insertions(+), 173 deletions(-) diff --git a/libraries/botbuilder-dialogs/src/choices/channel.ts b/libraries/botbuilder-dialogs/src/choices/channel.ts index f4b7dea315..03caf3058d 100644 --- a/libraries/botbuilder-dialogs/src/choices/channel.ts +++ b/libraries/botbuilder-dialogs/src/choices/channel.ts @@ -51,7 +51,6 @@ export function supportsCardActions(channelId: string, buttonCnt = 100): boolean case Channels.Directline: case Channels.DirectlineSpeech: case Channels.Webchat: - case Channels.Cortana: return buttonCnt <= 100; default: return false; @@ -60,15 +59,16 @@ export function supportsCardActions(channelId: string, buttonCnt = 100): boolean /** * @private - * @param channelId id of a channel + * @param _channelId id of a channel */ -export function hasMessageFeed(channelId: string): boolean { - switch (channelId) { - case Channels.Cortana: - return false; - default: - return true; - } +export function hasMessageFeed(_channelId: string): boolean { + // The removed 'cortana' channel was the only channel that returned false. + // This channel is no longer available for bot developers and was removed from + // the Channels enum while addressing issue #3603. + // Though it's marked as private in the docstring, the contents of channel.ts + // are publically available but not documented in the official reference docs. + // Thus, the method is retained. + return true; } /** diff --git a/libraries/botbuilder-dialogs/src/prompts/oauthPrompt.ts b/libraries/botbuilder-dialogs/src/prompts/oauthPrompt.ts index 9958397e6f..6829e69e79 100644 --- a/libraries/botbuilder-dialogs/src/prompts/oauthPrompt.ts +++ b/libraries/botbuilder-dialogs/src/prompts/oauthPrompt.ts @@ -641,7 +641,6 @@ export class OAuthPrompt extends Dialog { */ private static channelSupportsOAuthCard(channelId: string): boolean { switch (channelId) { - case Channels.Cortana: case Channels.Skype: case Channels.Skypeforbusiness: return false; diff --git a/libraries/botbuilder-dialogs/tests/choices_channel.test.js b/libraries/botbuilder-dialogs/tests/choices_channel.test.js index 163284c816..ae8216c2b4 100644 --- a/libraries/botbuilder-dialogs/tests/choices_channel.test.js +++ b/libraries/botbuilder-dialogs/tests/choices_channel.test.js @@ -1,103 +1,80 @@ const assert = require('assert'); const { Channels } = require('botbuilder-core'); -const { supportsSuggestedActions, supportsCardActions, hasMessageFeed, getChannelId } = require('../lib'); +const { getChannelId, hasMessageFeed, supportsSuggestedActions, supportsCardActions } = require('../'); -describe('channel methods', function() { +describe('channel methods', function () { this.timeout(5000); - it(`should return true for supportsSuggestedActions() with line and 13`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('line', 13); - assert(validNumOfSuggestedActions, `returned false.`); + it('should return true for supportsSuggestedActions() with line and 13', function () { + assert(supportsSuggestedActions(Channels.Line, 13)); }); - it(`should return false for supportsSuggestedActions() with line and 14`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('line', 14); - assert(validNumOfSuggestedActions === false, `returned true.`); + it('should return false for supportsSuggestedActions() with line and 14', function () { + assert.strictEqual(supportsSuggestedActions(Channels.Line, 14), false); }); - it(`should return true for supportsSuggestedActions() with skype and 10`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('skype', 10); - assert(validNumOfSuggestedActions, `returned false.`); + it('should return true for supportsSuggestedActions() with skype and 10', function () { + assert(supportsSuggestedActions(Channels.Skype, 10)); }); - it(`should return false for supportsSuggestedActions() with skype and 11`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('skype', 11); - assert(validNumOfSuggestedActions === false, `returned true.`); + it('should return false for supportsSuggestedActions() with skype and 11', function () { + assert.strictEqual(supportsSuggestedActions(Channels.Skype, 11), false); }); - it(`should return true for supportsSuggestedActions() with kik and 20`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('kik', 20); - assert(validNumOfSuggestedActions, `returned false.`); + it('should return true for supportsSuggestedActions() with kik and 20', function () { + assert(supportsSuggestedActions(Channels.Kik, 20)); }); - it(`should return false for supportsSuggestedActions() with kik and 21`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('kik', 21); - assert(validNumOfSuggestedActions === false, `returned true.`); + it('should return false for supportsSuggestedActions() with kik and 21', function () { + assert.strictEqual(supportsSuggestedActions(Channels.Kik, 21), false); }); - it(`should return true for supportsSuggestedActions() with emulator and 100`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('emulator', 100); - assert(validNumOfSuggestedActions, `returned false.`); + it('should return true for supportsSuggestedActions() with emulator and 100', function () { + assert(supportsSuggestedActions(Channels.Emulator, 100)); }); - it(`should return false for supportsSuggestedActions() with emulator and 101`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions('emulator', 101); - assert(validNumOfSuggestedActions === false, `returned true.`); + it('should return false for supportsSuggestedActions() with emulator and 101', function () { + assert.strictEqual(supportsSuggestedActions(Channels.Emulator, 101), false); }); - it(`should return true for supportsCardActions() with line and 99`, function() { - const validNumOfCardActions = supportsCardActions('line', 99); - assert(validNumOfCardActions, `returned false.`); + it('should return true for supportsCardActions() with line and 99', function () { + assert(supportsCardActions(Channels.Line, 99)); }); - it(`should return false for supportsCardActions() with line and 100`, function() { - const validNumOfCardActions = supportsCardActions('line', 100); - assert(validNumOfCardActions === false, `returned false.`); + it('should return false for supportsCardActions() with line and 100', function () { + assert.strictEqual(supportsCardActions(Channels.Line, 100), false); }); - it(`should return true for supportsCardActions() with cortana and 100`, function() { - const validNumOfCardActions = supportsCardActions('cortana', 100); - assert(validNumOfCardActions, `returned false.`); + it('should return false for supportsCardActions() with slack and 101', function () { + assert.strictEqual(supportsCardActions(Channels.Slack, 101), false); }); - it(`should return false for supportsCardActions() with slack and 101`, function() { - const validNumOfCardActions = supportsCardActions('slack', 101); - assert(validNumOfCardActions === false, `returned true.`); + it('should return true for supportsCardActions() with skype and 3', function () { + assert(supportsCardActions(Channels.Skype, 3)); }); - it(`should return true for supportsCardActions() with skype and 3`, function() { - const validNumOfCardActions = supportsCardActions('skype', 3); - assert(validNumOfCardActions, `returned false.`); + it('should return false for supportsCardActions() with skype and 5', function () { + assert.strictEqual(supportsCardActions(Channels.Skype, 5), false); }); - it(`should return false for supportsCardActions() with skype and 5`, function() { - const validNumOfCardActions = supportsCardActions('skype', 5); - assert(validNumOfCardActions === false, `returned true.`); + it('should return the channelId from context.activity.', function () { + assert.strictEqual(getChannelId({ activity: { channelId: Channels.Facebook } }), Channels.Facebook); }); - it(`should return false for hasMessageFeed() with cortana`, function() { - const hasFeed = hasMessageFeed('cortana'); - assert(hasFeed === false, `returned true.`); + it('should return true for any channel', function () { + assert(hasMessageFeed(Channels.Directline)); }); - it(`should return the channelId from context.activity.`, function() { - const channel = getChannelId({ activity: { channelId: 'facebook' } }); - assert(channel === 'facebook', `expected "facebook", instead received ${ channel }`); - }); - - it(`should return an empty string if context.activity.channelId is falsey.`, function() { - const channel = getChannelId({ activity: {} }); - assert(channel === '', `expected "", instead received ${ channel }`); + it('should return an empty string if context.activity.channelId is falsey.', function () { + assert.strictEqual(getChannelId({ activity: {} }), ''); }); // "directlinespeech" tests - it(`should return true for supportsSuggestedActions() with directlinespeech and 100`, function() { - const validNumOfSuggestedActions = supportsSuggestedActions(Channels.DirectlineSpeech, 100); - assert(validNumOfSuggestedActions, `returned false.`); + it('should return true for supportsSuggestedActions() with directlinespeech and 100', function () { + assert(supportsSuggestedActions(Channels.DirectlineSpeech, 100)); }); - it(`should return true for supportsCardActions() with directlinespeech and 100`, () => { - const validNumOfCardActions = supportsCardActions(Channels.DirectlineSpeech, 100); - assert(validNumOfCardActions, `returned false.`); + it('should return true for supportsCardActions() with directlinespeech and 100', function () { + assert(supportsCardActions(Channels.DirectlineSpeech, 100)); }); }); diff --git a/libraries/botbuilder-dialogs/tests/choices_choiceFactory.test.js b/libraries/botbuilder-dialogs/tests/choices_choiceFactory.test.js index 800027ed20..c6768f308b 100644 --- a/libraries/botbuilder-dialogs/tests/choices_choiceFactory.test.js +++ b/libraries/botbuilder-dialogs/tests/choices_choiceFactory.test.js @@ -1,50 +1,49 @@ const assert = require('assert'); +const { ActionTypes } = require('botbuilder-core'); const { ChoiceFactory } = require('../'); -const { ActionTypes, CardAction } = require('botbuilder-core'); function assertActivity(received, expected) { - assert(received, `Activity not returned.`); - for (let key in expected) { + assert(received); + for (const key in expected) { const v = received[key]; - assert(v !== undefined, `Activity.${ key } missing.`); + assert(v !== undefined, `Activity.${key} missing.`); const ev = expected[key]; - assert(typeof v === typeof ev, `Activity.${ key } has invalid type of '${ typeof v }'.`); + assert.strictEqual(typeof v, typeof ev); if (Array.isArray(ev)) { - assert(v.length === ev.length, `Activity.${ key } has invalid length of '${ v.length }'.`); - assert(JSON.stringify(v) === JSON.stringify(ev), `Activity.${ key } has invalid contents: ` + JSON.stringify(v)); + assert.strictEqual(v.length, ev.length); + assert.strictEqual(JSON.stringify(v), JSON.stringify(ev)); } else if (typeof ev === 'object') { - assert(JSON.stringify(v) === JSON.stringify(ev), `Activity.${ key } has invalid contents: ` + JSON.stringify(v)); + assert.strictEqual(JSON.stringify(v), JSON.stringify(ev)); } else { - assert(v === ev, `Activity.${ key } has invalid value of '${ v }'.`); + assert.strictEqual(v, ev); } } } const colorChoices = ['red', 'green', 'blue']; -const extraChoices = ['red', 'green', 'blue', 'alpha']; const choicesWithActionTitle = [ { value: 'red', action: { type: ActionTypes.ImBack, - title: 'Red Color' - } + title: 'Red Color', + }, }, { value: 'green', action: { type: ActionTypes.ImBack, - title: 'Green Color' - } + title: 'Green Color', + }, }, { value: 'blue', action: { type: ActionTypes.ImBack, - title: 'Blue Color' - } - } + title: 'Blue Color', + }, + }, ]; const choicesWithActionValue = [ @@ -52,89 +51,88 @@ const choicesWithActionValue = [ value: 'red', action: { type: ActionTypes.ImBack, - value: 'Red Color' - } + value: 'Red Color', + }, }, { value: 'green', action: { type: ActionTypes.ImBack, - value: 'Green Color' - } + value: 'Green Color', + }, }, { value: 'blue', action: { type: ActionTypes.ImBack, - value: 'Blue Color' - } - } + value: 'Blue Color', + }, + }, ]; const choicesWithEmptyActions = [ { value: 'red', - action: {} + action: {}, }, { value: 'green', - action: {} + action: {}, }, { value: 'blue', - action: {} - } + action: {}, + }, ]; const choicesWithPostBacks = [ { value: 'red', action: { - type: ActionTypes.PostBack - } + type: ActionTypes.PostBack, + }, }, { value: 'green', action: { - type: ActionTypes.PostBack - } + type: ActionTypes.PostBack, + }, }, { value: 'blue', action: { - type: ActionTypes.PostBack - } - } + type: ActionTypes.PostBack, + }, + }, ]; -function assertChoices(choices, actionValues, actionType = 'imBack') { - assert(choices.length === actionValues.length, 'test data prepared incorrectly.'); +function assertChoices(choices, actionValues, actionType = ActionTypes.ImBack) { + assert.strictEqual(choices.length, actionValues.length); for (let i = 0; i < choices.length; i++) { const choice = choices[i]; const val = actionValues[i]; - assert(choice.action.type === actionType, `Expected action.type === ${ actionType }, received ${ choice.action.type }`); - assert(choice.action.value === val, `Expected action.value === ${ val }, received ${ choice.action.value }`); - assert(choice.action.title === val, `Expected action.title === ${ val }, received ${ choice.action.title }`); - + assert.strictEqual(choice.action.type, actionType); + assert.strictEqual(choice.action.value, val); + assert.strictEqual(choice.action.title, val); } } -describe('The ChoiceFactory', function() { - it('should render choices inline.', () => { +describe('The ChoiceFactory', function () { + it('should render choices inline.', function () { const activity = ChoiceFactory.inline(colorChoices, 'select from:'); assertActivity(activity, { - text: `select from: (1) red, (2) green, or (3) blue` + text: `select from: (1) red, (2) green, or (3) blue`, }); }); - it('should render choices as a list.', () => { + it('should render choices as a list.', function () { const activity = ChoiceFactory.list(colorChoices, 'select from:'); assertActivity(activity, { - text: `select from:\n\n 1. red\n 2. green\n 3. blue` + text: `select from:\n\n 1. red\n 2. green\n 3. blue`, }); }); - it('should render choices as suggested actions.', () => { + it('should render choices as suggested actions.', function () { const activity = ChoiceFactory.suggestedAction(colorChoices, 'select from:'); assertActivity(activity, { text: `select from:`, @@ -142,74 +140,68 @@ describe('The ChoiceFactory', function() { actions: [ { type: 'imBack', value: 'red', title: 'red' }, { type: 'imBack', value: 'green', title: 'green' }, - { type: 'imBack', value: 'blue', title: 'blue' } - ] - } + { type: 'imBack', value: 'blue', title: 'blue' }, + ], + }, }); }); - it('should suggest the same action when a suggested action is provided', () => { + it('should suggest the same action when a suggested action is provided', function () { const activity = ChoiceFactory.suggestedAction([{ value: 'Signin', action: { type: ActionTypes.Signin } }]); - assert.ok(activity.suggestedActions.actions[0].type === ActionTypes.Signin, - `Expected the suggestion action to be ${ ActionTypes.Signin } but got: ${ activity.suggestedActions.actions[0].type }`); + assert.strictEqual(activity.suggestedActions.actions[0].type, ActionTypes.Signin); }); - it('should use hero cards for channels that do not support choices (Teams, Cortana)', () => { + it('should use hero cards for channels that do not support choices (i.e. Teams)', function () { const expectedActivity = { - 'type': 'message', - 'attachmentLayout': 'list', - 'attachments': [ + type: 'message', + attachmentLayout: 'list', + attachments: [ { - 'contentType': 'application/vnd.microsoft.card.hero', - 'content': { - 'text': 'select from:', - 'buttons': [ + contentType: 'application/vnd.microsoft.card.hero', + content: { + text: 'select from:', + buttons: [ { - 'title': 'red', - 'type': 'imBack', - 'value': 'red' + title: 'red', + type: 'imBack', + value: 'red', }, { - 'title': 'green', - 'type': 'imBack', - 'value': 'green' + title: 'green', + type: 'imBack', + value: 'green', }, { - 'title': 'blue', - 'type': 'imBack', - 'value': 'blue' - } - ] - } - } + title: 'blue', + type: 'imBack', + value: 'blue', + }, + ], + }, + }, ], - 'inputHint': 'expectingInput' + inputHint: 'expectingInput', }; - let activities = ChoiceFactory.forChannel('cortana', colorChoices, 'select from:'); - - - assertActivity(activities, expectedActivity); - - const choices = colorChoices.map(value => ({ + const choices = colorChoices.map((value) => ({ value, - action: { type: ActionTypes.ImBack } + action: { type: ActionTypes.ImBack }, })); - activities = ChoiceFactory.forChannel('msteams', choices, 'select from:'); + const activities = ChoiceFactory.forChannel('msteams', choices, 'select from:'); assertActivity(activities, expectedActivity); }); - it('should render an inline list based on title length, choice length and channel', () => { + it('should render an inline list based on title length, choice length and channel', function () { const activity = ChoiceFactory.forChannel('skypeforbusiness', colorChoices, 'select from:'); assertActivity(activity, { - 'type': 'message', - 'text': 'select from: (1) red, (2) green, or (3) blue', - 'inputHint': 'expectingInput' + type: 'message', + text: 'select from: (1) red, (2) green, or (3) blue', + inputHint: 'expectingInput', }); }); - it('should automatically choose render style based on channel type.', () => { + it('should automatically choose render style based on channel type.', function () { const activity = ChoiceFactory.forChannel('emulator', colorChoices, 'select from:'); assertActivity(activity, { text: `select from:`, @@ -217,37 +209,34 @@ describe('The ChoiceFactory', function() { actions: [ { type: 'imBack', value: 'red', title: 'red' }, { type: 'imBack', value: 'green', title: 'green' }, - { type: 'imBack', value: 'blue', title: 'blue' } - ] - } + { type: 'imBack', value: 'blue', title: 'blue' }, + ], + }, }); }); - it('should use action.title to populate action.value if action.value is falsey.', () => { + it('should use action.title to populate action.value if action.value is falsey.', function () { const preparedChoices = ChoiceFactory.toChoices(choicesWithActionTitle); assertChoices(preparedChoices, ['Red Color', 'Green Color', 'Blue Color']); }); - it('should use action.value to populate action.title if action.title is falsey.', () => { + it('should use action.value to populate action.title if action.title is falsey.', function () { const preparedChoices = ChoiceFactory.toChoices(choicesWithActionValue); assertChoices(preparedChoices, ['Red Color', 'Green Color', 'Blue Color']); }); - it('should use choice.value to populate action.title and action.value if both are missing.', () => { + it('should use choice.value to populate action.title and action.value if both are missing.', function () { const preparedChoices = ChoiceFactory.toChoices(choicesWithEmptyActions); assertChoices(preparedChoices, ['red', 'green', 'blue']); }); - it('should use provided ActionType.', () => { + it('should use provided ActionType.', function () { const preparedChoices = ChoiceFactory.toChoices(choicesWithPostBacks); assertChoices(preparedChoices, ['red', 'green', 'blue'], ActionTypes.PostBack); }); - it('should return a stylized list.', () => { - const listActivity = ChoiceFactory.forChannel('emulator', - ['choiceTitleOverTwentyChars'], - 'Test' - ); - assert(listActivity.text === 'Test\n\n 1. choiceTitleOverTwentyChars'); + it('should return a stylized list.', function () { + const listActivity = ChoiceFactory.forChannel('emulator', ['choiceTitleOverTwentyChars'], 'Test'); + assert.strictEqual(listActivity.text, 'Test\n\n 1. choiceTitleOverTwentyChars'); }); }); diff --git a/libraries/botframework-schema/src/index.ts b/libraries/botframework-schema/src/index.ts index 18cb0c8b16..a6b6ab9ee8 100644 --- a/libraries/botframework-schema/src/index.ts +++ b/libraries/botframework-schema/src/index.ts @@ -2137,7 +2137,6 @@ export enum SemanticActionStateTypes { export enum Channels { Alexa = 'alexa', Console = 'console', - Cortana = 'cortana', Directline = 'directline', DirectlineSpeech = 'directlinespeech', Email = 'email',