Skip to content

Commit

Permalink
Revert "Allow the Emulator user to specify a User ID (#1422)"
Browse files Browse the repository at this point in the history 
This reverts commit 2c6a4cf.
  • Loading branch information
Justin Wilaby committed Apr 15, 2019
1 parent 1d6480e commit 3b6ea7f
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 105 deletions.
33 changes: 2 additions & 31 deletions packages/app/client/src/ui/editor/appSettingsEditor/appSettingsEditor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,6 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
this.props.getFrameworkSettings();
}

private usingCustomId() {
return this.state.userGUID ? true : this.state.useCustomId;
}

public render(): JSX.Element {
const { state } = this;

Expand Down Expand Up @@ -171,7 +167,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
</Row>
</Column>
<Column className={[styles.rightColumn, styles.spacing].join(' ')}>
<SmallHeader>User settings</SmallHeader>
<SmallHeader>Auth</SmallHeader>
<Checkbox
className={styles.checkboxOverrides}
checked={state.use10Tokens}
Expand All @@ -180,6 +176,7 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
label="Use version 1.0 authentication tokens"
name="use10Tokens"
/>
<SmallHeader>Sign-in</SmallHeader>
<Checkbox
className={styles.checkboxOverrides}
checked={state.useCodeValidation}
Expand All @@ -188,30 +185,6 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
label="Use a sign-in verification code for OAuthCards"
name="useCodeValidation"
/>
<Checkbox
className={styles.checkboxOverrides}
checked={this.usingCustomId()}
onChange={this.onChangeCheckBox}
id="use-custom-id"
label="Use your own user ID to communicate with the bot"
name="useCustomId"
/>
<Row align={RowAlignment.Top}>
<label>User ID</label>
</Row>
<Row align={RowAlignment.Top}>
<TextField
className={styles.appSettingsInput}
inputContainerClassName={styles.inputContainer}
readOnly={false}
value={state.userGUID}
name="userGUID"
onChange={this.onInputChange}
disabled={!this.usingCustomId()}
required={this.usingCustomId()}
errorMessage={state.userGUID ? '' : 'Enter a user ID'}
/>
</Row>
<SmallHeader>Application Updates</SmallHeader>
<Checkbox
className={styles.checkboxOverrides}
Expand Down Expand Up @@ -254,8 +227,6 @@ export class AppSettingsEditor extends React.Component<AppSettingsEditorProps, A
const change = { [name]: checked };
this.setState(change);
this.updateDirtyFlag(change);

if (name === 'useCustomId' && checked === false) this.setState({ ['userGUID']: '' });
};

private onClickBrowse = async (): Promise<void> => {
Expand Down
2 changes: 0 additions & 2 deletions packages/app/client/src/ui/editor/emulator/emulator.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@
}

.restart-icon {
margin-left: 20px;

&::before { -webkit-mask: url(../../media/ic_refresh.svg); }
}
.save-transcript-icon {
Expand Down
52 changes: 24 additions & 28 deletions packages/app/client/src/ui/editor/emulator/emulator.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { SharedConstants } from '@bfemulator/app-shared';
import base64Url from 'base64url';

import { disable, enable } from '../../../data/action/presentationActions';
import { clearLog, newConversation, setInspectorObjects } from '../../../data/action/chatActions';
import { clearLog, newConversation, setInspectorObjects, updateChat } from '../../../data/action/chatActions';
import { updateDocument } from '../../../data/action/editorActions';

import { Emulator, EmulatorComponent, RestartConversationOptions } from './emulator';
Expand All @@ -63,10 +63,6 @@ jest.mock('../../../platform/commands/commandServiceImpl', () => ({
if (commandName === mockSharedConstants.Commands.Emulator.FeedTranscriptFromDisk) {
return Promise.resolve({ meta: 'some file info' });
}
if (commandName === mockSharedConstants.Commands.Settings.LoadAppSettings) {
return Promise.resolve({ framework: { userGUID: '' } });
}

return Promise.resolve();
},
},
Expand Down Expand Up @@ -297,9 +293,9 @@ describe('<Emulator/>', () => {
instance = wrapper.instance();
instance.onExportClick();

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

it('should start a new conversation', async () => {
Expand All @@ -314,7 +310,7 @@ describe('<Emulator/>', () => {
await instance.startNewConversation(undefined, false, false);

expect(mockUnsubscribe).toHaveBeenCalled();
expect(mockRemoteCallsMade).toHaveLength(5);
expect(mockRemoteCallsMade).toHaveLength(1);
expect(mockInitConversation).toHaveBeenCalledWith(
instance.props,
options,
Expand Down Expand Up @@ -354,9 +350,9 @@ describe('<Emulator/>', () => {
};
await instance.startNewConversation(undefined, false, true);

expect(mockRemoteCallsMade).toHaveLength(5);
expect(mockRemoteCallsMade[4].commandName).toBe(SharedConstants.Commands.Emulator.SetCurrentUser);
expect(mockRemoteCallsMade[4].args).toEqual([options.userId]);
expect(mockRemoteCallsMade).toHaveLength(2);
expect(mockRemoteCallsMade[1].commandName).toBe(SharedConstants.Commands.Emulator.SetCurrentUser);
expect(mockRemoteCallsMade[1].args).toEqual([options.userId]);
expect(mockInitConversation).toHaveBeenCalledWith(
instance.props,
options,
Expand All @@ -372,9 +368,9 @@ describe('<Emulator/>', () => {

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

Expand All @@ -385,9 +381,9 @@ describe('<Emulator/>', () => {

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

Expand Down Expand Up @@ -429,11 +425,11 @@ describe('<Emulator/>', () => {

await instance.startNewConversation(mockProps);

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

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

await instance.startNewConversation(mockProps);

expect(mockRemoteCallsMade).toHaveLength(7);
expect(mockRemoteCallsMade[5].commandName).toBe(SharedConstants.Commands.Emulator.NewTranscript);
expect(mockRemoteCallsMade[5].args).toEqual(['someUniqueId|transcript']);
expect(mockRemoteCallsMade[6].commandName).toBe(SharedConstants.Commands.Emulator.FeedTranscriptFromDisk);
expect(mockRemoteCallsMade[6].args).toEqual(['someConvoId', 'someBotId', 'someUserId', 'someDocId']);
expect(mockRemoteCallsMade).toHaveLength(3);
expect(mockRemoteCallsMade[1].commandName).toBe(SharedConstants.Commands.Emulator.NewTranscript);
expect(mockRemoteCallsMade[1].args).toEqual(['someUniqueId|transcript']);
expect(mockRemoteCallsMade[2].commandName).toBe(SharedConstants.Commands.Emulator.FeedTranscriptFromDisk);
expect(mockRemoteCallsMade[2].args).toEqual(['someConvoId', 'someBotId', 'someUserId', 'someDocId']);
expect(mockDispatch).toHaveBeenCalledWith(updateDocument('someDocId', { meta: 'some file info' }));
});
});
58 changes: 29 additions & 29 deletions packages/app/client/src/ui/editor/emulator/emulator.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@

import { createDirectLine } from 'botframework-webchat';
import { Activity, uniqueId, uniqueIdv4 } from '@bfemulator/sdk-shared';
import { Splitter } from '@bfemulator/ui-react';
import { Splitter, SplitButton } from '@bfemulator/ui-react';
import base64Url from 'base64url';
import { IEndpointService } from 'botframework-config/lib/schema';
import * as React from 'react';
import { connect } from 'react-redux';
import { BehaviorSubject } from 'rxjs';
import { newNotification, Notification, SharedConstants, FrameworkSettings } from '@bfemulator/app-shared';
import { newNotification, Notification, SharedConstants } from '@bfemulator/app-shared';

import * as ChatActions from '../../../data/action/chatActions';
import { updateDocument } from '../../../data/action/editorActions';
Expand Down Expand Up @@ -112,6 +112,9 @@ export class EmulatorComponent extends React.Component<EmulatorProps, {}> {

componentWillMount() {
window.addEventListener('keydown', this.keyboardEventListener);
if (this.shouldStartNewConversation()) {
this.startNewConversation();
}
}

componentDidMount() {
Expand All @@ -125,7 +128,18 @@ export class EmulatorComponent extends React.Component<EmulatorProps, {}> {
}

componentWillReceiveProps(nextProps: EmulatorProps) {
const { props, keyboardEventListener } = this;
const { props, keyboardEventListener, startNewConversation } = this;
const { document = {} } = props;
const { document: nextDocument = {} } = nextProps;

const documentOrUserIdChanged =
(!nextDocument.directLine && document.documentId !== nextDocument.documentId) ||
document.userId !== nextDocument.userId;

if (documentOrUserIdChanged) {
startNewConversation(nextProps).catch();
}

const switchedDocuments = props.activeDocumentId !== nextProps.activeDocumentId;
const switchedToThisDocument = nextProps.activeDocumentId === props.documentId;

Expand All @@ -141,18 +155,12 @@ export class EmulatorComponent extends React.Component<EmulatorProps, {}> {

startNewConversation = async (
props: EmulatorProps = this.props,
requireNewConvoId?: boolean,
requireNewUserId?: boolean
requireNewConvoId: boolean = false,
requireNewUserId: boolean = false
): Promise<any> => {
if (props.document.subscription) {
props.document.subscription.unsubscribe();
}
if (!requireNewUserId) {
requireNewUserId = false;
}
if (!requireNewConvoId) {
requireNewConvoId = false;
}
const selectedActivity$ = new BehaviorSubject<Activity | null>({});
const subscription = selectedActivity$.subscribe(activity => {
if (activity && activity.showInInspector) {
Expand All @@ -165,13 +173,11 @@ export class EmulatorComponent extends React.Component<EmulatorProps, {}> {
const conversationId = requireNewConvoId
? `${uniqueId()}|${props.mode}`
: props.document.conversationId || `${uniqueId()}|${props.mode}`;
const framework: FrameworkSettings = await CommandServiceImpl.remoteCall(
SharedConstants.Commands.Settings.LoadAppSettings
);
const stableId = framework.userGUID || props.document.userId;
const userId = requireNewUserId ? uniqueIdv4() : stableId;

await CommandServiceImpl.remoteCall(SharedConstants.Commands.Emulator.SetCurrentUser, userId);
const userId = requireNewUserId ? uniqueIdv4() : props.document.userId;
if (requireNewUserId) {
await CommandServiceImpl.remoteCall(SharedConstants.Commands.Emulator.SetCurrentUser, userId);
}

const options = {
conversationId,
Expand Down Expand Up @@ -289,18 +295,12 @@ export class EmulatorComponent extends React.Component<EmulatorProps, {}> {
{this.props.mode === 'livechat' && (
<div className={styles.header}>
<ToolBar>
<button
className={`${styles.restartIcon} ${styles.toolbarIcon || ''}`}
onClick={() => this.onStartOverClick(SameUserId)}
>
Restart with Same User Id
</button>
<button
className={`${styles.restartIcon} ${styles.toolbarIcon || ''}`}
onClick={() => this.onStartOverClick(NewUserId)}
>
Restart with New User Id
</button>
<SplitButton
defaultLabel="Restart conversation"
buttonClass={styles.restartIcon}
options={[NewUserId, SameUserId]}
onClick={this.onStartOverClick}
/>
<button
className={`${styles.saveTranscriptIcon} ${styles.toolbarIcon || ''}`}
onClick={this.onExportClick}
Expand Down
1 change: 1 addition & 0 deletions packages/app/client/src/ui/editor/emulator/toolbar/toolbar.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
flex-wrap: nowrap;
align-items: center;
height: 29px;
padding-left: 12px;
background-color: var(--toolbar-bg);
border-top: var(--toolbar-border);
border-bottom: var(--toolbar-border-bottom);
Expand Down
12 changes: 2 additions & 10 deletions packages/app/main/src/commands/settingsCommands.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { setFramework } from '../settingsData/actions/frameworkActions';

import { registerCommands } from './settingsCommands';

const mockSettings = { framework: { ngrokPath: 'path/to/ngrok.exe', userGUID: 'aRandomUser', useCustomId: true } };
const mockSettings = { framework: { ngrokPath: 'path/to/ngrok.exe' } };
let mockDispatch;
jest.mock('../settingsData/store', () => ({
get dispatch() {
Expand Down Expand Up @@ -78,14 +78,6 @@ describe('The settings commands', () => {
const { handler } = mockRegistry.getCommand(SharedConstants.Commands.Settings.LoadAppSettings);
const appSettings = await handler();

expect(appSettings.ngrokPath).toBe(mockSettings.framework.ngrokPath);
});

it('should load the app settings from the store', async () => {
const { handler } = mockRegistry.getCommand(SharedConstants.Commands.Settings.LoadAppSettings);
const appSettings = await handler();

expect(appSettings.userGUID).toBe(mockSettings.framework.userGUID);
expect(appSettings.useCustomId).toBe(mockSettings.framework.useCustomId);
expect(appSettings).toBe(mockSettings.framework);
});
});
5 changes: 0 additions & 5 deletions packages/app/shared/src/types/serverSettingsTypes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ export interface FrameworkSettings {
collectUsageData?: boolean;
// Digest of k/v pairs for integrity
hash?: string;
// GUID set by the user
userGUID?: string;
// use custom user id
useCustomId?: boolean;
}

export interface WindowStateSettings {
Expand Down Expand Up @@ -136,7 +132,6 @@ export const frameworkDefault: FrameworkSettings = {
usePrereleases: false,
autoUpdate: true,
collectUsageData: false,
userGUID: '',
};

export const windowStateDefault: WindowStateSettings = {
Expand Down

0 comments on commit 3b6ea7f

Please sign in to comment.