diff --git a/packages/app/client/src/data/sagas/endpointSagas.spec.ts b/packages/app/client/src/data/sagas/endpointSagas.spec.ts index 72a4571f2..f6577c56e 100644 --- a/packages/app/client/src/data/sagas/endpointSagas.spec.ts +++ b/packages/app/client/src/data/sagas/endpointSagas.spec.ts @@ -81,9 +81,10 @@ describe('The endpoint sagas', () => { describe(' openEndpointContextMenu', () => { const menuItems = [ - { 'id': 'edit', 'label': 'Edit settings' }, - { 'id': 'open', 'label': 'Open in emulator' }, - { 'id': 'forget', 'label': 'Remove' } + { label: 'Edit settings', id: 'edit' }, + { label: 'Open in emulator', id: 'open' }, + { label: 'Open in portal', id: 'absLink', enabled: jasmine.any(Boolean) }, + { label: 'Remove', id: 'forget' } ]; const { DisplayContextMenu, ShowMessageBox } = SharedConstants.Commands.Electron; diff --git a/packages/app/client/src/data/sagas/endpointSagas.ts b/packages/app/client/src/data/sagas/endpointSagas.ts index 43ba63381..f42a835ea 100644 --- a/packages/app/client/src/data/sagas/endpointSagas.ts +++ b/packages/app/client/src/data/sagas/endpointSagas.ts @@ -31,11 +31,13 @@ // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. // -import { IEndpointService } from 'botframework-config/lib/schema'; +import { SharedConstants } from '@bfemulator/app-shared'; +import { IBotService, IEndpointService, ServiceTypes } from 'botframework-config/lib/schema'; import { ComponentClass } from 'react'; -import { call, ForkEffect, takeEvery, takeLatest } from 'redux-saga/effects'; +import { call, ForkEffect, put, select, takeEvery, takeLatest } from 'redux-saga/effects'; import { CommandServiceImpl } from '../../platform/commands/commandServiceImpl'; import { DialogService } from '../../ui/dialogs/service'; +import { openServiceDeepLink } from '../action/connectedServiceActions'; import { EndpointEditorPayload, EndpointServiceAction, @@ -44,28 +46,43 @@ import { OPEN_ENDPOINT_CONTEXT_MENU, OPEN_ENDPOINT_DEEP_LINK } from '../action/endpointServiceActions'; -import { SharedConstants } from '@bfemulator/app-shared'; +import { RootState } from '../store'; + +const getConnectedAbs = (state: RootState, endpointAppId: string) => { + return (state.bot.activeBot.services || []).find(service => { + return service.type === ServiceTypes.Bot && (service as IBotService).appId === endpointAppId; + }); +}; function* launchEndpointEditor(action: EndpointServiceAction): IterableIterator { const { endpointEditorComponent, endpointService = {} } = action.payload; - const servicesToUpdate = yield DialogService - .showDialog>(endpointEditorComponent, { - endpointService - }); + const servicesToUpdate = yield DialogService.showDialog, IEndpointService[]> + (endpointEditorComponent, { endpointService }); + if (servicesToUpdate) { + const { AddOrUpdateService, RemoveService } = SharedConstants.Commands.Bot; let i = servicesToUpdate.length; while (i--) { const service = servicesToUpdate[i]; - yield CommandServiceImpl.remoteCall(SharedConstants.Commands.Bot.AddOrUpdateService, service.type, service); + let shouldBeRemoved = false; + if (service.type === ServiceTypes.Bot) { + // Since we could end up with an invalid ABS + // naively validate and remove it if all fields are missing + const { serviceName, resourceGroup, subscriptionId, tenantId } = service as IBotService; + shouldBeRemoved = !serviceName && !resourceGroup && !subscriptionId && !tenantId; + } + yield CommandServiceImpl.remoteCall(shouldBeRemoved ? RemoveService : AddOrUpdateService, service.type, service); } } } function* openEndpointContextMenu(action: EndpointServiceAction) : IterableIterator { + const connectedAbs = yield select(getConnectedAbs, action.payload.endpointService.appId); const menuItems = [ { label: 'Edit settings', id: 'edit' }, { label: 'Open in emulator', id: 'open' }, + { label: 'Open in portal', id: 'absLink', enabled: !!connectedAbs }, { label: 'Remove', id: 'forget' } ]; const { DisplayContextMenu } = SharedConstants.Commands.Electron; @@ -79,6 +96,10 @@ function* openEndpointContextMenu(action: EndpointServiceAction // do something with password from dialog) */ - showDialog(dialog: T, props: {} = {}): Promise { + showDialog(dialog: T, props: {} = {}): Promise { if (!this._hostElement) { return new Promise((resolve) => resolve(null)); } diff --git a/packages/app/main/src/botHelpers.spec.ts b/packages/app/main/src/botHelpers.spec.ts index db9252951..ca84eeb3a 100644 --- a/packages/app/main/src/botHelpers.spec.ts +++ b/packages/app/main/src/botHelpers.spec.ts @@ -13,7 +13,7 @@ // MIT License: // Permission is hereby granted, free of charge, to any person obtaining // a copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including +// Software), to deal in the Software without restriction, including // without limitation the rights to use, copy, modify, merge, publish, // distribute, sublicense, and/or sell copies of the Software, and to // permit persons to whom the Software is furnished to do so, subject to @@ -22,7 +22,7 @@ // The above copyright notice and this permission notice shall be // included in all copies or substantial portions of the Software. // -// THE SOFTWARE IS PROVIDED ""AS IS"", WITHOUT WARRANTY OF ANY KIND, +// THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND // NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE @@ -47,7 +47,8 @@ jest.mock('./botData/store', () => ({ botFiles: [ { path: 'path1', displayName: 'name1', secret: '' }, { path: 'path2', displayName: 'name2', secret: '' }, - { path: 'path3', displayName: 'name3', secret: '' } + { path: 'path3', displayName: 'name3', secret: '' }, + { path: 'path4', displayName: 'name4', secret: 'ffsafsdfdsa' } ] } }), @@ -75,11 +76,12 @@ import { removeBotFromList, cloneBot, toSavableBot, - promptForSecretAndRetry + promptForSecretAndRetry, loadBotWithRetry, saveBot } from './botHelpers'; -describe('botHelpers tests', () => { - test('getActiveBot()', () => { +describe('The botHelpers', () => { + + it('getActiveBot() should retrieve the active bot', () => { let activeBot = getActiveBot(); expect(activeBot).toEqual({ name: 'someBot', @@ -90,31 +92,32 @@ describe('botHelpers tests', () => { }); }); - test('getBotInfoByPath()', () => { + it('getBotInfoByPath() should get the bot info matching the specified path', () => { const info = getBotInfoByPath('path2'); expect(info).toEqual({ path: 'path2', displayName: 'name2', secret: '' }); }); - test('pathExistsInRecentBots()', () => { + it('pathExistsInRecentBots() should determine if the specified path exists in the recent bot list', () => { const pathExists = pathExistsInRecentBots('path1'); expect(pathExists).toBe(true); }); - test(`removeBotFromList()`, () => { + it(`removeBotFromList() should remove the bot from the list based on the specified path`, async () => { const spy = jest.spyOn(mainWindow.commandService, 'remoteCall'); - removeBotFromList('path3'); + await removeBotFromList('path3'); // should have sync'd up list with remaining 2 bot entries (3rd was removed) expect(spy).toHaveBeenCalledWith( SharedConstants.Commands.Bot.SyncBotList, [ { path: 'path1', displayName: 'name1', secret: '' }, - { path: 'path2', displayName: 'name2', secret: '' } + { path: 'path2', displayName: 'name2', secret: '' }, + {displayName: 'name4', path: 'path4', secret: 'ffsafsdfdsa'} ] ); }); - test('cloneBot()', () => { + it('cloneBot() should clone the specified bot as expected', () => { const bot1 = null; expect(cloneBot(bot1)).toBe(null); @@ -130,9 +133,9 @@ describe('botHelpers tests', () => { expect(cloneBot(bot2)).toEqual(bot2); }); - test('toSavableBot()', () => { + it('toSavableBot() should convert the specified bot to a savable instance', () => { const bot1 = null; - expect(() => toSavableBot(bot1)).toThrowError('Cannot convert falsy bot to savable bot.'); + expect(() => toSavableBot(bot1)).toThrowError('Cannot convert null bot to savable bot.'); const bot2: BotConfigWithPath = BotConfigWithPathImpl.fromJSON({ version: '', @@ -157,7 +160,7 @@ describe('botHelpers tests', () => { expect(savableBot.padlock).not.toEqual(secret); }); - test('promptForSecretAndRetry()', async () => { + it('promptForSecretAndRetry() should prompt the user for the bot secret', async () => { mainWindow.commandService.remoteCall = jest.fn() .mockImplementationOnce(() => Promise.resolve(null)) .mockImplementation(() => Promise.resolve('secret')); @@ -173,4 +176,59 @@ describe('botHelpers tests', () => { expect(e.code).toBe('ENOENT'); } }); + + it('saveBot() should save a bot', async () => { + let saved = false; + const fromJSONSpy = jest.spyOn(BotConfiguration, 'fromJSON').mockReturnValue({ + internal: {}, + validateSecret: () => true, + save: async () => { + saved = true; + } + }); + await saveBot({ + path: 'path4' + } as any); + expect(saved).toBeTruthy(); + }); + + describe('loadBotWithRetry()', () => { + + it('should prompt the user for the secret and retry if no secret was given for an encrypted bot', async () => { + const botConfigLoadSpy = jest.spyOn(BotConfiguration, 'load').mockResolvedValue({ padlock: '55sdgfd' }); + const result = await loadBotWithRetry('path'); + expect(botConfigLoadSpy).toHaveBeenCalledWith('path', undefined); + + expect(result).toEqual({ + description: '', + name: '', + overrides: null, + padlock: '55sdgfd', + path: 'path', + services: [], + version: '2.0' + }); + }); + + it('should update the secret when the specified secret does not match the one on record', async () => { + const botConfigLoadSpy = jest.spyOn(BotConfiguration, 'load').mockResolvedValue({ padlock: 'newSecret' }); + const remoteCallSpy = jest.spyOn(mainWindow.commandService, 'remoteCall').mockResolvedValue('newSecret'); + const result = await loadBotWithRetry('path1'); + expect(botConfigLoadSpy).toHaveBeenCalledWith('path1', undefined); + expect(result).toEqual({ + description: '', + name: '', + overrides: null, + padlock: 'newSecret', + path: 'path1', + services: [], + version: '2.0' + }); + expect(remoteCallSpy).toHaveBeenCalledWith('bot:list:sync', [ + { displayName: 'name1', path: 'path1', secret: 'newSecret' }, + { displayName: 'name2', path: 'path2', secret: '' }, + { displayName: 'name3', path: 'path3', secret: '' }, + { path: 'path4', displayName: 'name4', secret: 'ffsafsdfdsa' }]); + }); + }); }); diff --git a/packages/app/main/src/botHelpers.ts b/packages/app/main/src/botHelpers.ts index 9ab0b673a..26ab79b9f 100644 --- a/packages/app/main/src/botHelpers.ts +++ b/packages/app/main/src/botHelpers.ts @@ -121,7 +121,7 @@ export async function promptForSecretAndRetry(botPath: string): Promise ({ pathExistsInRecentBots: () => true, getBotInfoByPath: () => ({ secret: 'secret' }), loadBotWithRetry: () => mockBot, - getActiveBot: () => mockBot + getActiveBot: () => mockBot, + removeBotFromList: async () => true } )); + jest.mock('../utils/ensureStoragePath', () => ({ ensureStoragePath: () => '' })); + jest.mock('../emulator', () => ({ emulator: { framework: { @@ -51,7 +57,7 @@ jest.mock('../emulator', () => ({ })); const mockCommandRegistry = new CommandRegistryImpl(); const mockBot = BotConfigWithPathImpl.fromJSON({ - 'path': 'some/path', + 'path': 'some/path.bot', 'name': 'AuthBot', 'description': '', 'padlock': '', @@ -66,6 +72,7 @@ const mockBot = BotConfigWithPathImpl.fromJSON({ } ] } as any); + registerCommands(mockCommandRegistry); jest.mock('../main', () => ({ mainWindow: { @@ -76,7 +83,7 @@ jest.mock('../main', () => ({ } })); -const mockOn = { on: () => mockOn }; +const mockOn = { on: () => mockOn, close: () => void 0 }; jest.mock('chokidar', () => ({ watch: () => ({ on: () => mockOn @@ -165,4 +172,101 @@ describe('The botCommands', () => { savedBot, savedBot.getPath()); }); + + it('should throw when updating a service fails', async () => { + const serviceToUpdate = mockBot.services[0]; + jest.spyOn(store.getStore(), 'dispatch').mockImplementationOnce(() => { + throw new Error(''); + }); + const { handler } = mockCommandRegistry.getCommand(Bot.AddOrUpdateService); + let threw = false; + try { + await handler(serviceToUpdate.type, serviceToUpdate); + } catch (e) { + threw = true; + } + jest.restoreAllMocks(); + expect(threw).toBeTruthy(); + }); + + it('should throw when saving a new service and the service types do not match', async () => { + const serviceToSave = JSON.parse(JSON.stringify(mockBot.services[0])); + serviceToSave.id = null; + serviceToSave.type = ServiceTypes.AppInsights; + const { handler } = mockCommandRegistry.getCommand(Bot.AddOrUpdateService); + let threw = false; + try { + await handler(ServiceTypes.Luis, serviceToSave); + } catch (e) { + threw = true; + expect(e.message).toEqual('serviceType does not match'); + } + expect(threw).toBeTruthy(); + }); + + it('should remove a service as expected', async () => { + const serviceToRemove = mockBot.services[0]; + const remoteCallSpy = jest.spyOn(mainWindow.commandService, 'remoteCall'); + const { handler } = mockCommandRegistry.getCommand(Bot.RemoveService); + await handler(serviceToRemove.type, serviceToRemove.id); + const savedBot = mockBotConfig.fromJSON(store.getStore().getState().bot.activeBot); + expect(savedBot.services.length).toBe(0); + + expect(remoteCallSpy).toHaveBeenCalledWith(SharedConstants.Commands.Bot.SetActive, + savedBot, + savedBot.getPath()); + }); + + it('should throw when removing a service fails', async () => { + const serviceToRemove = mockBot.services[0]; + jest.spyOn(store.getStore(), 'dispatch').mockImplementationOnce(() => { + throw new Error(''); + }); + const { handler } = mockCommandRegistry.getCommand(Bot.RemoveService); + let threw = false; + try { + await handler(serviceToRemove.type, serviceToRemove.id); + } catch (e) { + threw = true; + } + jest.restoreAllMocks(); + expect(threw).toBeTruthy(); + }); + + it('should patch the bots list and watch for chat and transcript changes', async () => { + const mockBotInfo = { + path: 'this/is/my.json', + displayName: 'AuthBot', + secret: 'secret' + }; + const transcriptWatchSpy = jest.spyOn(transcriptsWatcher, 'watch'); + const chatWatcherSpy = jest.spyOn(chatWatcher, 'watch'); + + const { handler } = mockCommandRegistry.getCommand(SharedConstants.Commands.Bot.PatchBotList); + await handler(mockBotInfo.path, mockBotInfo); + expect(transcriptWatchSpy).toHaveBeenCalledWith('this/is/transcripts'); + expect(chatWatcherSpy).toHaveBeenCalledWith('this/is/dialogs'); + }); + + it('should remove a bot from the list', async () => { + const callSpy = jest.spyOn(mainWindow.commandService, 'call').mockResolvedValue(true); + const { handler } = mockCommandRegistry.getCommand(SharedConstants.Commands.Bot.RemoveFromBotList); + const removeBotFromListSpy = jest.spyOn((helpers as any).default, 'removeBotFromList').mockResolvedValue(true); + await handler('some/bot/path.json'); + expect(callSpy).toHaveBeenCalledWith('shell:showExplorer-message-box', true, { + buttons: ['Cancel', 'OK'], + cancelId: 0, + defaultId: 1, + message: 'Remove Bot some/bot/path.json from bots list. Are you sure?', + type: 'question' + }); + expect(removeBotFromListSpy).toHaveBeenCalledWith('some/bot/path.json'); + }); + + it('should close the bot', async () => { + const { handler } = mockCommandRegistry.getCommand(SharedConstants.Commands.Bot.Close); + const dispatchSpy = jest.spyOn(getStore(), 'dispatch'); + await handler(); + expect(dispatchSpy).toHaveBeenCalledWith(BotActions.close()); + }); }); diff --git a/packages/app/main/src/commands/botCommands.ts b/packages/app/main/src/commands/botCommands.ts index 676f0890a..d1f1a7c48 100644 --- a/packages/app/main/src/commands/botCommands.ts +++ b/packages/app/main/src/commands/botCommands.ts @@ -31,6 +31,14 @@ // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. // +import { BotInfo, getBotDisplayName, SharedConstants } from '@bfemulator/app-shared'; +import { BotConfigWithPath, CommandRegistryImpl, mergeEndpoints, uniqueId } from '@bfemulator/sdk-shared'; +import { BotConfigurationBase } from 'botframework-config/lib'; +import { IConnectedService, IEndpointService, ServiceTypes } from 'botframework-config/lib/schema'; +import * as path from 'path'; +import * as BotActions from '../botData/actions/botActions'; +import { setActive } from '../botData/actions/botActions'; +import { getStore } from '../botData/store'; import { getActiveBot, getBotInfoByPath, @@ -41,16 +49,8 @@ import { saveBot, toSavableBot } from '../botHelpers'; -import * as BotActions from '../botData/actions/botActions'; -import { setActive } from '../botData/actions/botActions'; -import { BotConfigWithPath, CommandRegistryImpl, mergeEndpoints, uniqueId } from '@bfemulator/sdk-shared'; -import { BotInfo, getBotDisplayName, SharedConstants } from '@bfemulator/app-shared'; -import { mainWindow } from '../main'; import { emulator } from '../emulator'; -import { IConnectedService, IEndpointService, ServiceTypes } from 'botframework-config/lib/schema'; -import { BotConfigurationBase } from 'botframework-config/lib'; -import * as path from 'path'; -import { getStore } from '../botData/store'; +import { mainWindow } from '../main'; import { botProjectFileWatcher, chatWatcher, transcriptsWatcher } from '../watchers'; /** Registers bot commands */ @@ -260,14 +260,20 @@ export function registerCommands(commandRegistry: CommandRegistryImpl) { // --------------------------------------------------------------------------- // Removes an msbot service entry. - commandRegistry.registerCommand(Bot.RemoveService, async (serviceType: ServiceTypes, serviceId: string) => { + commandRegistry.registerCommand(Bot.RemoveService, async (serviceType: ServiceTypes, serviceOrId: any) => { const activeBot = getActiveBot(); const botInfo = activeBot && getBotInfoByPath(activeBot.path); if (botInfo) { const botConfig = toSavableBot(activeBot, botInfo.secret); - botConfig.disconnectService(serviceId); + const id = typeof serviceOrId === 'string' ? serviceOrId : serviceOrId.id; + botConfig.disconnectService(id); try { - await botConfig.save(botInfo.secret); + await saveBot(botConfig); + getStore().dispatch(setActive(botConfig)); + await mainWindow.commandService.remoteCall( + SharedConstants.Commands.Bot.SetActive, + botConfig, + botConfig.getPath()); } catch (e) { console.error(`bot:remove-service: Error trying to save bot: ${e}`); throw e;