Skip to content

Commit

Permalink
Revised user id logic.
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyanziano committed May 22, 2019
1 parent 83f54b9 commit a562689
Show file tree
Hide file tree
Showing 11 changed files with 197 additions and 92 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.md
Expand Up @@ -18,8 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [client] Fixed issue where selecting an autocomplete result with the mouse was losing focus in PR [1554](https://github.com/microsoft/BotFramework-Emulator/pull/1554)
- [client] Fixed issue where tab icon for sidecar debugging docs page was shrinking in PR [1557](https://github.com/microsoft/BotFramework-Emulator/pull/1557)
- [build] Fixed issue where the NSIS installer was requiring admin permissions to run, causing auto update to fail when attempting to install in PR [1562](https://github.com/microsoft/BotFramework-Emulator/pull/1562)
[main] Added type 'button' to cancel button #1504 in PR [1551](https://github.com/microsoft/BotFramework-Emulator/pull/1551)
[luis] Fixed the spinner issue when publishin LUIS after training #1572 in PR [1582](https://github.com/microsoft/BotFramework-Emulator/pull/1582)
- [main] Added type 'button' to cancel button #1504 in PR [1551](https://github.com/microsoft/BotFramework-Emulator/pull/1551)
- [luis] Fixed the spinner issue when publishin LUIS after training #1572 in PR [1582](https://github.com/microsoft/BotFramework-Emulator/pull/1582)
- [client / main] Corrected user id logic in PR [1590](https://github.com/microsoft/BotFramework-Emulator/pull/1590)


## v4.4.0 - 2019 - 05 - 03
## Added
Expand Down
16 changes: 15 additions & 1 deletion packages/app/client/src/commands/emulatorCommands.spec.ts
Expand Up @@ -40,8 +40,10 @@ import { bot } from '../data/reducer/bot';
import { chat } from '../data/reducer/chat';
import { clientAwareSettings } from '../data/reducer/clientAwareSettingsReducer';
import { editor } from '../data/reducer/editor';
import { framework } from '../data/reducer/frameworkSettingsReducer';
import { RootState } from '../data/store';
import { CommandServiceImpl } from '../platform/commands/commandServiceImpl';
import { frameworkSettingsChanged } from '../data/action/frameworkSettingsActions';

import { registerCommands } from './emulatorCommands';

Expand All @@ -65,7 +67,7 @@ describe('The emulator commands', () => {
});

beforeEach(() => {
mockStore = createStore(combineReducers({ bot, chat, clientAwareSettings, editor }));
mockStore = createStore(combineReducers({ bot, chat, clientAwareSettings, editor, framework }));
mockStore.dispatch(
clientAwareSettingsChanged({
users: { currentUserId: '1234' },
Expand All @@ -82,10 +84,22 @@ describe('The emulator commands', () => {
const documentId = handler(mockEndpoint, false);
const state: RootState = mockStore.getState();
const documentIds = Object.keys(state.chat.chats);
const document = state.chat.chats[documentId];
expect(document.userId).toEqual('1234');
expect(documentIds.length).toBe(1);
expect(state.editor.editors.primary.activeDocumentId).toBe(documentId);
});

it('should open a new emulator tabbed document for an endpoint and use the custom user id', () => {
let state: RootState = mockStore.getState();
mockStore.dispatch(frameworkSettingsChanged({ ...state.framework, userGUID: 'customUserId' }));
const { handler } = registry.getCommand(SharedConstants.Commands.Emulator.NewLiveChat);
const documentId = handler(mockEndpoint, false);
state = mockStore.getState();
const document = state.chat.chats[documentId];
expect(document.userId).toEqual('customUserId');
});

it('should set the active tab of an existing chat', () => {
const { handler } = registry.getCommand(SharedConstants.Commands.Emulator.NewLiveChat);
const documentId = handler(mockEndpoint, false);
Expand Down
3 changes: 2 additions & 1 deletion packages/app/client/src/commands/emulatorCommands.ts
Expand Up @@ -77,11 +77,12 @@ export function registerCommands(commandRegistry: CommandRegistryImpl) {
if (!documentId) {
documentId = uniqueId();
const { currentUserId } = state.clientAwareSettings.users;
const customUserId = state.framework.userGUID;
const action = ChatActions.newChat(documentId, mode, {
botId: 'bot',
endpointId: endpoint.id,
endpointUrl: endpoint.endpoint,
userId: currentUserId,
userId: customUserId || currentUserId,
conversationId,
// directLine: createDirectLine({
// secret: base64Url.encode(JSON.stringify({ conversationId, endpointId: endpoint.id })),
Expand Down
36 changes: 0 additions & 36 deletions packages/app/client/src/conversationDefault.ts

This file was deleted.

90 changes: 70 additions & 20 deletions packages/app/client/src/data/sagas/botSagas.spec.ts
Expand Up @@ -33,7 +33,7 @@

import { newNotification, SharedConstants, DebugMode } from '@bfemulator/app-shared';
import { BotConfigWithPath, ConversationService } from '@bfemulator/sdk-shared';
import { call, put, select, takeEvery, takeLatest } from 'redux-saga/effects';
import { call, put, takeEvery, takeLatest } from 'redux-saga/effects';

import { ActiveBotHelper } from '../../ui/helpers/activeBotHelper';
import {
Expand Down Expand Up @@ -86,6 +86,7 @@ describe('The botSagas', () => {
beforeEach(() => {
mockRemoteCommandsCalled = [];
mockLocalCommandsCalled = [];
ConversationService.startConversation = jest.fn().mockResolvedValue(true);
});

it('should initialize the root saga', () => {
Expand Down Expand Up @@ -137,33 +138,74 @@ describe('The botSagas', () => {
});

it('should open a bot from a url', () => {
const mockState = {
clientAwareSettings: {
serverUrl: 'http://localhost:3000',
users: {
currentUserId: '1',
usersById: { '1': {} },
},
},
};
const gen = openBotViaUrl(
openBotViaUrlAction({
appPassword: 'password',
appId: '1234abcd',
endpoint: 'http://localhost/api/messages',
})
);
const io = select(() => void 0);
io.SELECT.selector = jasmine.any(Function) as any;
gen.next();
// select serverUrl
const selectServerUrl = gen.next().value as any;
expect(selectServerUrl).toEqual(io);
// select user
const selectUser = gen.next(selectServerUrl.SELECT.selector(mockState)).value as any;
expect(selectUser).toEqual(io);
// call ConversationService.startConversation
jest.spyOn(ConversationService, 'startConversation').mockResolvedValue({ ok: true });
expect(gen.next(selectUser.SELECT.selector(mockState)).value).not.toBeNull();
gen.next('www.serverurl.com');
// select custom user id
gen.next('');
// select users
const users = { currentUserId: 'user1', usersById: { user1: {} } };
gen.next(users);
// startConversation
gen.next({ ok: true, json: async () => null });
// select debug mode
const callToSaveUrl = gen.next(DebugMode.Normal).value;
expect(callToSaveUrl).toEqual(
call(
[CommandServiceImpl, CommandServiceImpl.remoteCall],
SharedConstants.Commands.Settings.SaveBotUrl,
'http://localhost/api/messages'
)
);
// the saga should be finished
expect(gen.next().done).toBe(true);
});

it('should open a bot from a url with the custom user id', () => {
const gen = openBotViaUrl(
openBotViaUrlAction({
appPassword: 'password',
appId: '1234abcd',
endpoint: 'http://localhost/api/messages',
})
);
gen.next();
// select serverUrl
gen.next('www.serverurl.com');
// select custom user id
gen.next('customUserId');
// select users
const users = { currentUserId: 'user1', usersById: { user1: {} } };
const callToSetCurrentUser = gen.next(users).value;
expect(callToSetCurrentUser).toEqual(
call(
[CommandServiceImpl, CommandServiceImpl.remoteCall],
SharedConstants.Commands.Emulator.SetCurrentUser,
'customUserId'
)
);
// call to set current user
gen.next();
// startConversation
gen.next({ ok: true, json: async () => null });
// select debug mode
const callToSaveUrl = gen.next(DebugMode.Normal).value;
expect(callToSaveUrl).toEqual(
call(
[CommandServiceImpl, CommandServiceImpl.remoteCall],
SharedConstants.Commands.Settings.SaveBotUrl,
'http://localhost/api/messages'
)
);
// the saga should be finished
expect(gen.next().done).toBe(true);
});

it('should send a notification if opening a bot from a URL fails', () => {
Expand All @@ -177,6 +219,8 @@ describe('The botSagas', () => {
gen.next();
// select serverUrl
gen.next('www.serverurl.com');
// select custom user id
gen.next('');
// select users
const users = { currentUserId: 'user1', usersById: { user1: {} } };
gen.next(users);
Expand Down Expand Up @@ -206,6 +250,8 @@ describe('The botSagas', () => {
gen.next();
// select serverUrl
gen.next('www.serverurl.com');
// select custom user id
gen.next('');
// select users
const users = { currentUserId: 'user1', usersById: { user1: {} } };
gen.next(users);
Expand Down Expand Up @@ -252,6 +298,8 @@ describe('The botSagas', () => {
gen.next();
// select serverUrl
gen.next('www.serverurl.com');
// select custom user id
gen.next('');
// select users
const users = { currentUserId: 'user1', usersById: { user1: {} } };
gen.next(users);
Expand Down Expand Up @@ -281,6 +329,8 @@ describe('The botSagas', () => {
gen.next();
// select serverUrl
gen.next('www.serverurl.com');
// select custom user id
gen.next('');
// select users
const users = { currentUserId: 'user1', usersById: { user1: {} } };
gen.next(users);
Expand Down
11 changes: 10 additions & 1 deletion packages/app/client/src/data/sagas/botSagas.ts
Expand Up @@ -68,8 +68,17 @@ export function* openBotViaUrl(action: BotAction<Partial<StartConversationParams
const serverUrl = yield select((state: RootState) => state.clientAwareSettings.serverUrl);
if (!action.payload.user) {
// If no user is provided, select the current user
const customUserId = yield select((state: RootState) => state.framework.userGUID);
const users: UserSettings = yield select((state: RootState) => state.clientAwareSettings.users);
action.payload.user = users.usersById[users.currentUserId];
action.payload.user = customUserId || users.usersById[users.currentUserId];
if (customUserId) {
action.payload.user = customUserId;
yield call(
[CommandServiceImpl, CommandServiceImpl.remoteCall],
SharedConstants.Commands.Emulator.SetCurrentUser,
customUserId
);
}
}
let error;
try {
Expand Down
58 changes: 35 additions & 23 deletions packages/app/client/src/ui/editor/emulator/emulator.spec.tsx
Expand Up @@ -288,9 +288,9 @@ describe('<EmulatorContainer/>', () => {
it('should export a transcript', async () => {
await instance.onExportTranscriptClick();

expect(mockRemoteCallsMade).toHaveLength(2);
expect(mockRemoteCallsMade[1].commandName).toBe(SharedConstants.Commands.Emulator.SaveTranscriptToFile);
expect(mockRemoteCallsMade[1].args).toEqual([32, 'convo1']);
expect(mockRemoteCallsMade).toHaveLength(3);
expect(mockRemoteCallsMade[2].commandName).toBe(SharedConstants.Commands.Emulator.SaveTranscriptToFile);
expect(mockRemoteCallsMade[2].args).toEqual([32, 'convo1']);
});

it('should report a notification when exporting a transcript fails', async () => {
Expand All @@ -312,7 +312,7 @@ describe('<EmulatorContainer/>', () => {
};
await instance.startNewConversation(undefined, true, true);

expect(mockRemoteCallsMade).toHaveLength(2);
expect(mockRemoteCallsMade).toHaveLength(3);
expect(initConversationSpy).toHaveBeenCalledWith(instance.props, options);
});

Expand All @@ -330,7 +330,7 @@ describe('<EmulatorContainer/>', () => {
conversationId: 'someUniqueId|livechat',
conversationMode: instance.props.mode,
endpointId: instance.props.endpointId,
userId: 'genericUserId',
userId: 'someUserId',
};
await instance.startNewConversation(undefined, true, false);

Expand All @@ -349,8 +349,8 @@ describe('<EmulatorContainer/>', () => {
};
await instance.startNewConversation(undefined, false, true);

expect(mockRemoteCallsMade).toHaveLength(2);
expect(mockRemoteCallsMade[1].commandName).toBe(SharedConstants.Commands.Settings.LoadAppSettings);
expect(mockRemoteCallsMade).toHaveLength(3);
expect(mockRemoteCallsMade[0].commandName).toBe(SharedConstants.Commands.Settings.LoadAppSettings);
expect(mockInitConversation).toHaveBeenCalledWith(instance.props, options);
});

Expand All @@ -361,9 +361,15 @@ describe('<EmulatorContainer/>', () => {

expect(mockDispatch).toHaveBeenCalledWith(clearLog('doc1'));
expect(mockDispatch).toHaveBeenCalledWith(setInspectorObjects('doc1', []));
expect(mockRemoteCallsMade).toHaveLength(2);
expect(mockRemoteCallsMade[1].commandName).toBe(SharedConstants.Commands.Telemetry.TrackEvent);
expect(mockRemoteCallsMade[1].args).toEqual(['conversation_restart', { userId: 'new' }]);
expect(mockRemoteCallsMade).toHaveLength(3);
expect(mockRemoteCallsMade.some(cmd => cmd.commandName === SharedConstants.Commands.Telemetry.TrackEvent)).toBe(
true
);
expect(
mockRemoteCallsMade.some(cmd => {
return cmd.args[0] === 'conversation_restart' && cmd.args[1].userId === 'new';
})
).toBe(true);
expect(mockStartNewConversation).toHaveBeenCalledWith(undefined, true, true);
});

Expand All @@ -374,9 +380,15 @@ describe('<EmulatorContainer/>', () => {

expect(mockDispatch).toHaveBeenCalledWith(clearLog('doc1'));
expect(mockDispatch).toHaveBeenCalledWith(setInspectorObjects('doc1', []));
expect(mockRemoteCallsMade).toHaveLength(2);
expect(mockRemoteCallsMade[1].commandName).toBe(SharedConstants.Commands.Telemetry.TrackEvent);
expect(mockRemoteCallsMade[1].args).toEqual(['conversation_restart', { userId: 'same' }]);
expect(mockRemoteCallsMade).toHaveLength(3);
expect(mockRemoteCallsMade.some(cmd => cmd.commandName === SharedConstants.Commands.Telemetry.TrackEvent)).toBe(
true
);
expect(
mockRemoteCallsMade.some(cmd => {
return cmd.args[0] === 'conversation_restart' && cmd.args[1].userId === 'same';
})
).toBe(true);
expect(mockStartNewConversation).toHaveBeenCalledWith(undefined, true, false);
});

Expand Down Expand Up @@ -416,11 +428,11 @@ describe('<EmulatorContainer/>', () => {

await instance.startNewConversation(mockProps);

expect(mockRemoteCallsMade).toHaveLength(4);
expect(mockRemoteCallsMade[2].commandName).toBe(SharedConstants.Commands.Emulator.NewTranscript);
expect(mockRemoteCallsMade[2].args).toEqual(['someUniqueId|transcript']);
expect(mockRemoteCallsMade[3].commandName).toBe(SharedConstants.Commands.Emulator.FeedTranscriptFromMemory);
expect(mockRemoteCallsMade[3].args).toEqual(['someConvoId', 'someBotId', 'someUserId', []]);
expect(mockRemoteCallsMade).toHaveLength(6);
expect(mockRemoteCallsMade[4].commandName).toBe(SharedConstants.Commands.Emulator.NewTranscript);
expect(mockRemoteCallsMade[4].args).toEqual(['someUniqueId|transcript']);
expect(mockRemoteCallsMade[5].commandName).toBe(SharedConstants.Commands.Emulator.FeedTranscriptFromMemory);
expect(mockRemoteCallsMade[5].args).toEqual(['someConvoId', 'someBotId', 'someUserId', []]);
});

it('should start a new conversation from transcript on disk', async () => {
Expand All @@ -440,11 +452,11 @@ describe('<EmulatorContainer/>', () => {

await instance.startNewConversation(mockProps);

expect(mockRemoteCallsMade).toHaveLength(4);
expect(mockRemoteCallsMade[2].commandName).toBe(SharedConstants.Commands.Emulator.NewTranscript);
expect(mockRemoteCallsMade[2].args).toEqual(['someUniqueId|transcript']);
expect(mockRemoteCallsMade[3].commandName).toBe(SharedConstants.Commands.Emulator.FeedTranscriptFromDisk);
expect(mockRemoteCallsMade[3].args).toEqual(['someConvoId', 'someBotId', 'someUserId', 'someDocId']);
expect(mockRemoteCallsMade).toHaveLength(6);
expect(mockRemoteCallsMade[4].commandName).toBe(SharedConstants.Commands.Emulator.NewTranscript);
expect(mockRemoteCallsMade[4].args).toEqual(['someUniqueId|transcript']);
expect(mockRemoteCallsMade[5].commandName).toBe(SharedConstants.Commands.Emulator.FeedTranscriptFromDisk);
expect(mockRemoteCallsMade[5].args).toEqual(['someConvoId', 'someBotId', 'someUserId', 'someDocId']);
expect(mockDispatch).toHaveBeenCalledWith(updateDocument('someDocId', { meta: 'some file info' }));
});
});

0 comments on commit a562689

Please sign in to comment.