diff --git a/Composer/packages/client/__tests__/store/action/lgHandlers.test.ts b/Composer/packages/client/__tests__/store/action/lgHandlers.test.ts new file mode 100644 index 0000000000..6cd5f6fab2 --- /dev/null +++ b/Composer/packages/client/__tests__/store/action/lgHandlers.test.ts @@ -0,0 +1,71 @@ +import { isLgActivity, copyLgTemplate } from '../../../src/store/action/lgHandlers'; + +describe('lgHandlers', () => { + describe('#isLgActivity', () => { + it('can handle empty input', () => { + expect(isLgActivity('')).toBeFalsy(); + }); + + it('can identify correct templates', () => { + expect(isLgActivity('[bfdactivity-1234]')).toBeTruthy(); + expect(isLgActivity('[bfdprompt-1234]')).toBeTruthy(); + expect(isLgActivity('[bfdinvalidPrompt-1234]')).toBeTruthy(); + expect(isLgActivity('[bfdunrecognizedPrompt-1234]')).toBeTruthy(); + }); + + it('can identify invalid templates', () => { + expect(isLgActivity('any string')).toBeFalsy(); + + expect(isLgActivity('bfdactivity-1234')).toBeFalsy(); + expect(isLgActivity('[bfdactivity-1234')).toBeFalsy(); + expect(isLgActivity('bfdactivity-1234')).toBeFalsy(); + expect(isLgActivity('[bfdactivity-abc]')).toBeFalsy(); + + expect(isLgActivity('[abfdactivity-1234]')).toBeFalsy(); + expect(isLgActivity('[abfdactivity]')).toBeFalsy(); + expect(isLgActivity('[bfdactivity-]')).toBeFalsy(); + }); + }); + + describe('#copyLgTemplate', () => { + const lgApi = { + getLgTemplates: (id, activity) => Promise.resolve([{ Name: 'bfdactivity-1234', Body: 'Hello' }]), + updateLgTemplate: (id, newId, newContent) => Promise.resolve(true), + }; + + it('can skip invalid input params', async () => { + expect(await copyLgTemplate('common', '', 'newId', lgApi)).toEqual(''); + expect(await copyLgTemplate('common', 'wrong', 'newId', lgApi)).toEqual('wrong'); + expect(await copyLgTemplate('common', 'wrong', 'newId', null as any)).toEqual('wrong'); + }); + + it('can copy existing template to a new template', async () => { + expect(await copyLgTemplate('common', '[bfdactivity-1234]', '[bfdactivity-5678]', lgApi)).toEqual( + '[bfdactivity-5678]' + ); + }); + + it('can handle non-existing template', async () => { + expect(await copyLgTemplate('common', '[bfdactivity-4321]', '[bfdactivity-5678]', lgApi)).toEqual( + '[bfdactivity-4321]' + ); + }); + + it('can recover from API error', async () => { + // getLgTemplates error + expect( + await copyLgTemplate('common', '[bfdactivity-1234]', 'bfdactivity-5678', { + ...lgApi, + getLgTemplates: () => Promise.reject(), + }) + ).toEqual('[bfdactivity-1234]'); + + expect( + await copyLgTemplate('common', '[bfdactivity-1234]', 'bfdactivity-5678', { + ...lgApi, + updateLgTemplate: () => Promise.reject(), + }) + ).toEqual('Hello'); + }); + }); +}); diff --git a/Composer/packages/client/src/extension-container/ExtensionContainer.tsx b/Composer/packages/client/src/extension-container/ExtensionContainer.tsx index 9bb140d325..52ce274e94 100644 --- a/Composer/packages/client/src/extension-container/ExtensionContainer.tsx +++ b/Composer/packages/client/src/extension-container/ExtensionContainer.tsx @@ -2,6 +2,7 @@ import React, { useState, useEffect } from 'react'; import { initializeIcons } from 'office-ui-fabric-react'; import { LuFile, ShellData } from 'shared'; +import { copyLgTemplate } from '../store/action/lgHandlers'; import ApiClient from '../messenger/ApiClient'; import getEditor from './EditorMap'; @@ -95,6 +96,13 @@ const shellApi = { }); }, + copyLgTemplate: (id: string, templateName: string, newTemplateName: string) => { + return copyLgTemplate(id, templateName, newTemplateName, { + getLgTemplates: shellApi.getLgTemplates, + updateLgTemplate: shellApi.updateLgTemplate, + }); + }, + createDialog: () => { return apiClient.apiCall('createDialog'); }, diff --git a/Composer/packages/client/src/store/action/lgHandlers.ts b/Composer/packages/client/src/store/action/lgHandlers.ts new file mode 100644 index 0000000000..675cdea138 --- /dev/null +++ b/Composer/packages/client/src/store/action/lgHandlers.ts @@ -0,0 +1,42 @@ +const TEMPLATE_PATTERN = /^\[(bfd.+-\d+)\]$/; +export function isLgActivity(activity: string) { + return activity && TEMPLATE_PATTERN.test(activity); +} + +export async function copyLgTemplate( + lgFileName: string, + templateNameToCopy: string, + newTemplateName: string, + lgApi: { + getLgTemplates: (lgFileName: string, templateName: string) => Promise; + updateLgTemplate: (lgFileName: string, newTemplateName: string, newContent: string) => Promise; + } +): Promise { + if (!templateNameToCopy) return ''; + if (!isLgActivity(templateNameToCopy) || !lgApi) return templateNameToCopy; + + const { getLgTemplates, updateLgTemplate } = lgApi; + if (!getLgTemplates) return templateNameToCopy; + + let rawLg: any[] = []; + try { + rawLg = await getLgTemplates(lgFileName, templateNameToCopy); + } catch (error) { + return templateNameToCopy; + } + + const currentLg = rawLg.find(lg => `[${lg.Name}]` === templateNameToCopy); + + if (currentLg) { + // Create new lg activity. + const newLgContent = currentLg.Body; + try { + const newTemplateId = (TEMPLATE_PATTERN.exec(newTemplateName) || [])[1]; + await updateLgTemplate(lgFileName, newTemplateId, newLgContent); + return newTemplateName; + } catch (e) { + return newLgContent; + } + } + return templateNameToCopy; +} diff --git a/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx b/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx index 8262f011ba..ea0bbe40c0 100644 --- a/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx +++ b/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx @@ -39,15 +39,12 @@ export const ObiEditor: FC = ({ }): JSX.Element | null => { let divRef; - const { focusedId, focusedEvent, updateLgTemplate, getLgTemplates, removeLgTemplate } = useContext( - NodeRendererContext - ); + const { focusedId, focusedEvent, removeLgTemplate, copyLgTemplate } = useContext(NodeRendererContext); const [clipboardContext, setClipboardContext] = useState({ clipboardActions: [], setClipboardActions: actions => setClipboardContext({ ...clipboardContext, clipboardActions: actions }), }); - const lgApi = { getLgTemplates, removeLgTemplate, updateLgTemplate }; const dispatchEvent = (eventName: NodeEventTypes, eventData: any): any => { let handler; switch (eventName) { @@ -108,7 +105,12 @@ export const ObiEditor: FC = ({ case NodeEventTypes.Insert: if (eventData.$type === 'PASTE') { handler = e => { - pasteNodes(data, e.id, e.position, clipboardContext.clipboardActions, lgApi).then(dialog => { + // Bind 'common' as lg template file name to align with 'removeLgTemplate' usage. + const externalCopyApi = { + copyLgTemplate: (templateNameToCopy: string, newTemplateName: string) => + copyLgTemplate('common', templateNameToCopy, newTemplateName), + }; + pasteNodes(data, e.id, e.position, clipboardContext.clipboardActions, externalCopyApi).then(dialog => { onChange(dialog); }); }; diff --git a/Composer/packages/extensions/visual-designer/src/index.tsx b/Composer/packages/extensions/visual-designer/src/index.tsx index 76277a7fb8..e51f632a19 100644 --- a/Composer/packages/extensions/visual-designer/src/index.tsx +++ b/Composer/packages/extensions/visual-designer/src/index.tsx @@ -48,6 +48,7 @@ const VisualDesigner: React.FC = ({ updateLgTemplate, getLgTemplates, removeLgTemplate, + copyLgTemplate, undo, redo, } = shellApi; @@ -62,6 +63,7 @@ const VisualDesigner: React.FC = ({ updateLgTemplate: updateLgTemplate, getLgTemplates: getLgTemplates, removeLgTemplate: removeLgTemplate, + copyLgTemplate: copyLgTemplate, }); useEffect(() => { diff --git a/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts b/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts index 384a9d5b2d..d43a15efdb 100644 --- a/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts +++ b/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts @@ -1,5 +1,4 @@ import React from 'react'; -import { string } from 'prop-types'; interface LgTemplate { Name: string; @@ -12,5 +11,6 @@ export const NodeRendererContext = React.createContext({ focusedTab: '', getLgTemplates: (_id: string, _templateName: string) => Promise.resolve([] as LgTemplate[]), removeLgTemplate: (_id: string, _templateName: string) => Promise.resolve(), - updateLgTemplate: (_id: string, _templateName: string, _template: string) => Promise.resolve('' as string), + updateLgTemplate: (_id: string, _templateName: string, _template: string) => Promise.resolve(''), + copyLgTemplate: (_id: string, _templateName: string, _newTemplateName: string) => Promise.resolve(''), }); diff --git a/Composer/packages/extensions/visual-designer/src/utils/jsonTracker.ts b/Composer/packages/extensions/visual-designer/src/utils/jsonTracker.ts index 842216468b..d4ad29848b 100644 --- a/Composer/packages/extensions/visual-designer/src/utils/jsonTracker.ts +++ b/Composer/packages/extensions/visual-designer/src/utils/jsonTracker.ts @@ -196,7 +196,15 @@ export function appendNodesAfter(inputDialog, targetId, newNodes) { return dialog; } -export async function pasteNodes(inputDialog, arrayPath, arrayIndex, newNodes, lgApi) { +export async function pasteNodes( + inputDialog: any, + arrayPath: string, + arrayIndex: number, + newNodes: any[], + externalApi: { + copyLgTemplate: (srcTemplateName: string, newTemplateName: string) => Promise; + } +) { if (!Array.isArray(newNodes) || newNodes.length === 0) { return inputDialog; } @@ -211,7 +219,7 @@ export async function pasteNodes(inputDialog, arrayPath, arrayIndex, newNodes, l // Deep copy nodes with external resources const copiedNodes = await Promise.all( newNodes.map(async x => { - const node = await deepCopyAction(x, lgApi); + const node = await deepCopyAction(x, externalApi); return node; }) ); diff --git a/Composer/packages/lib/shared/__tests__/copyUtils.test.ts b/Composer/packages/lib/shared/__tests__/utils/copyUtils.test.ts similarity index 60% rename from Composer/packages/lib/shared/__tests__/copyUtils.test.ts rename to Composer/packages/lib/shared/__tests__/utils/copyUtils.test.ts index 5938033901..a0b10fb6dd 100644 --- a/Composer/packages/lib/shared/__tests__/copyUtils.test.ts +++ b/Composer/packages/lib/shared/__tests__/utils/copyUtils.test.ts @@ -1,26 +1,11 @@ -import { copyAdaptiveAction } from '../src/copyUtils'; +import { copyAdaptiveAction } from '../../src/utils/copyUtils'; describe('copyAdaptiveAction', () => { - const lgTemplate = [{ Name: 'bfdactivity-1234', Body: '-hello' }]; const externalApi = { updateDesigner: data => { data.$designer = { id: '5678' }; }, - lgApi: { - getLgTemplates: (fileId, activityId) => { - return Promise.resolve(lgTemplate); - }, - updateLgTemplate: (filedId, activityId, activityBody) => { - return Promise.resolve(true); - }, - }, - }; - const externalApiWithFailure = { - ...externalApi, - lgApi: { - ...externalApi.lgApi, - updateLgTemplate: () => Promise.reject(), - }, + copyLgTemplate: (templateToCopy: string, newTemplateName: string) => Promise.resolve(newTemplateName), }; it('should return {} when input is invalid', async () => { @@ -53,12 +38,6 @@ describe('copyAdaptiveAction', () => { $designer: { id: '5678' }, activity: '[bfdactivity-5678]', }); - - expect(await copyAdaptiveAction(sendActivity, externalApiWithFailure)).toEqual({ - $type: 'Microsoft.SendActivity', - $designer: { id: '5678' }, - activity: '-hello', - }); }); it('can copy TextInput', async () => { @@ -72,7 +51,8 @@ describe('copyAdaptiveAction', () => { alwaysPrompt: false, allowInterruptions: 'true', outputFormat: 'none', - prompt: '[bfdactivity-1234]', + prompt: '[bfdprompt-1234]', + invalidPrompt: '[bfdinvalidPrompt-1234]', }; expect(await copyAdaptiveAction(promptText, externalApi)).toEqual({ @@ -84,19 +64,8 @@ describe('copyAdaptiveAction', () => { alwaysPrompt: false, allowInterruptions: 'true', outputFormat: 'none', - prompt: '[bfdactivity-5678]', - }); - - expect(await copyAdaptiveAction(promptText, externalApiWithFailure)).toEqual({ - $type: 'Microsoft.TextInput', - $designer: { - id: '5678', - }, - maxTurnCount: 3, - alwaysPrompt: false, - allowInterruptions: 'true', - outputFormat: 'none', - prompt: '-hello', + prompt: '[bfdprompt-5678]', + invalidPrompt: '[bfdinvalidPrompt-5678]', }); }); }); diff --git a/Composer/packages/lib/shared/__tests__/utils/walkAdaptiveAction.test.ts b/Composer/packages/lib/shared/__tests__/utils/walkAdaptiveAction.test.ts new file mode 100644 index 0000000000..78fe0f61fa --- /dev/null +++ b/Composer/packages/lib/shared/__tests__/utils/walkAdaptiveAction.test.ts @@ -0,0 +1,38 @@ +import { walkAdaptiveAction } from '../../src/utils/walkAdaptiveAction'; + +describe('#walkAdaptiveAction', () => { + const action = { + $type: 'Microsoft.IfCondition', + actions: [{ $type: 'Microsoft.SendActivity' }], + elseActions: [ + { + $type: 'Microsoft.Foreach', + actions: [{ $type: 'Microsoft.SendActivity' }], + }, + ], + }; + + it('can walk every child Adaptive Action node', async () => { + let actionCount = 0; + const counter: any = () => { + actionCount += 1; + }; + await walkAdaptiveAction(action, counter); + expect(actionCount).toEqual(4); + }); + + it('can walk every child Adaptive Action node async', async () => { + let actionCount = 0; + const counter = () => { + return new Promise(resolve => { + setTimeout(() => { + actionCount += 1; + resolve(); + }, 10); + }); + }; + + await walkAdaptiveAction(action, counter); + expect(actionCount).toEqual(4); + }); +}); diff --git a/Composer/packages/lib/shared/src/copyUtils.ts b/Composer/packages/lib/shared/src/copyUtils.ts deleted file mode 100644 index f65e59144d..0000000000 --- a/Composer/packages/lib/shared/src/copyUtils.ts +++ /dev/null @@ -1,116 +0,0 @@ -const NestedFieldNames = { - Actions: 'actions', - ElseActions: 'elseActions', - DefaultCase: 'default', - Cases: 'cases', -}; - -const DEFAULT_CHILDREN_KEYS = [NestedFieldNames.Actions]; -const childrenMap = { - ['Microsoft.IfCondition']: [NestedFieldNames.Actions, NestedFieldNames.ElseActions], - ['Microsoft.SwitchCondition']: [NestedFieldNames.Cases, NestedFieldNames.DefaultCase], -}; - -/** - * Considering that an Adaptive Action could be nested with other actions, - * for example, the IfCondition and SwitchCondition and Foreach, we need - * this helper to visit all possible action nodes recursively. - * - * @param {any} input The input Adaptive Action which has $type field. - * @param {function} visitor The callback function called on each action node. - */ -async function walkAdaptiveAction(input: any, visitor: (data: any) => Promise) { - if (!input || !input.$type) return; - - await visitor(input); - - let childrenKeys = DEFAULT_CHILDREN_KEYS; - if (input.$type && childrenMap[input.$type]) { - childrenKeys = childrenMap[input.$type]; - } - - for (const childrenKey of childrenKeys) { - const children = input[childrenKey]; - if (Array.isArray(children)) { - Promise.all(children.map(async x => await walkAdaptiveAction(x, visitor))); - } - } -} - -function isLgActivity(activity: string) { - return activity && (activity.includes('bfdactivity-') || activity.includes('bfdprompt-')); -} - -async function copyLgActivity(activity: string, designerId: string, lgApi: any): Promise { - if (!activity) return ''; - if (!isLgActivity(activity) || !lgApi) return activity; - - const { getLgTemplates, updateLgTemplate } = lgApi; - if (!getLgTemplates) return activity; - - let rawLg: any[] = []; - try { - rawLg = await getLgTemplates('common', activity); - } catch (error) { - return activity; - } - - const currentLg = rawLg.find(lg => `[${lg.Name}]` === activity); - - if (currentLg) { - // Create new lg activity. - const newLgContent = currentLg.Body; - const newLgId = `bfdactivity-${designerId}`; - try { - await updateLgTemplate('common', newLgId, newLgContent); - return `[${newLgId}]`; - } catch (e) { - return newLgContent; - } - } - return activity; -} - -const overrideLgActivity = async (data, { lgApi }) => { - data.activity = await copyLgActivity(data.activity, data.$designer.id, lgApi); -}; - -const overrideLgPrompt = async (data, { lgApi }) => { - data.prompt = await copyLgActivity(data.prompt, data.$designer.id, lgApi); -}; - -// TODO: use $type from SDKTypes (after solving circular import issue). -const OverriderByType = { - 'Microsoft.SendActivity': overrideLgActivity, - 'Microsoft.AttachmentInput': overrideLgPrompt, - 'Microsoft.ConfirmInput': overrideLgPrompt, - 'Microsoft.DateTimeInput': overrideLgPrompt, - 'Microsoft.NumberInput': overrideLgPrompt, - 'Microsoft.OAuthInput': overrideLgPrompt, - 'Microsoft.TextInput': overrideLgPrompt, - 'Microsoft.ChoiceInput': overrideLgPrompt, -}; - -const needsOverride = data => !!(data && OverriderByType[data.$type]); - -export async function copyAdaptiveAction(data, externalApi) { - if (!data || !data.$type) return {}; - - // Deep copy the original data. - const copy = JSON.parse(JSON.stringify(data)); - - const { updateDesigner } = externalApi; - // Create copy handler for rewriting fields which need to be handled specially. - const copyHandler = async data => { - updateDesigner(data); - if (needsOverride(data)) { - const overrider = OverriderByType[data.$type]; - await overrider(data, externalApi); - } - }; - - // Walk action and rewrite needs copy fields - await walkAdaptiveAction(copy, copyHandler); - - return copy; -} diff --git a/Composer/packages/lib/shared/src/dialogFactory.ts b/Composer/packages/lib/shared/src/dialogFactory.ts index 1612f442b5..316ea361b0 100644 --- a/Composer/packages/lib/shared/src/dialogFactory.ts +++ b/Composer/packages/lib/shared/src/dialogFactory.ts @@ -1,7 +1,7 @@ import nanoid from 'nanoid/generate'; import { appschema } from './appschema'; -import { copyAdaptiveAction } from './copyUtils'; +import { copyAdaptiveAction } from './utils/copyUtils'; interface DesignerAttributes { name: string; @@ -96,10 +96,11 @@ const updateDesigner = data => { data.$designer = $designer; }; -// TODO: lgApi should also be included in shared lib instead of pass it in -// since it's already used by Shell, Visual and Form. -export const deepCopyAction = async (data, lgApi) => { - return await copyAdaptiveAction(data, { lgApi, updateDesigner }); +export const deepCopyAction = async ( + data, + externalCopyApi: { copyLgTemplate: (templateName: string, newTemplateName: string) => Promise } +) => { + return await copyAdaptiveAction(data, { copyLgTemplate: externalCopyApi.copyLgTemplate, updateDesigner }); }; export const seedNewDialog = ( diff --git a/Composer/packages/lib/shared/src/utils/copyUtils.ts b/Composer/packages/lib/shared/src/utils/copyUtils.ts new file mode 100644 index 0000000000..f3baab43f9 --- /dev/null +++ b/Composer/packages/lib/shared/src/utils/copyUtils.ts @@ -0,0 +1,54 @@ +import cloneDeep from 'lodash.clonedeep'; + +import { walkAdaptiveAction } from './walkAdaptiveAction'; + +const overrideLgActivity = async (data, { copyLgTemplate }) => { + const newLgId = `[bfdactivity-${data.$designer.id}]`; + data.activity = await copyLgTemplate(data.activity, newLgId); +}; + +const LG_FIELDS = ['prompt', 'unrecognizedPrompt', 'invalidPrompt', 'defaultValueResponse']; +const overrideLgPrompt = async (data, { copyLgTemplate }) => { + for (const promptFieldKey of LG_FIELDS) { + const existingActivity = data[promptFieldKey]; + const newLgId = `[bfd${promptFieldKey}-${data.$designer.id}]`; + if (existingActivity) { + data[promptFieldKey] = await copyLgTemplate(existingActivity, newLgId); + } + } +}; + +// TODO: use $type from SDKTypes (after solving circular import issue). +const OverriderByType = { + 'Microsoft.SendActivity': overrideLgActivity, + 'Microsoft.AttachmentInput': overrideLgPrompt, + 'Microsoft.ConfirmInput': overrideLgPrompt, + 'Microsoft.DateTimeInput': overrideLgPrompt, + 'Microsoft.NumberInput': overrideLgPrompt, + 'Microsoft.OAuthInput': overrideLgPrompt, + 'Microsoft.TextInput': overrideLgPrompt, + 'Microsoft.ChoiceInput': overrideLgPrompt, +}; + +const needsOverride = data => !!(data && OverriderByType[data.$type]); + +export async function copyAdaptiveAction(data, externalApi: { updateDesigner: Function; copyLgTemplate: Function }) { + if (!data || !data.$type) return {}; + + const copy = cloneDeep(data); + + const { updateDesigner } = externalApi; + // Create copy handler for rewriting fields which need to be handled specially. + const copyHandler = async data => { + updateDesigner(data); + if (needsOverride(data)) { + const overrider = OverriderByType[data.$type]; + await overrider(data, externalApi); + } + }; + + // Walk action and rewrite needs copy fields + await walkAdaptiveAction(copy, copyHandler); + + return copy; +} diff --git a/Composer/packages/lib/shared/src/utils/walkAdaptiveAction.ts b/Composer/packages/lib/shared/src/utils/walkAdaptiveAction.ts new file mode 100644 index 0000000000..afb288a78e --- /dev/null +++ b/Composer/packages/lib/shared/src/utils/walkAdaptiveAction.ts @@ -0,0 +1,38 @@ +const NestedFieldNames = { + Actions: 'actions', + ElseActions: 'elseActions', + DefaultCase: 'default', + Cases: 'cases', +}; + +const DEFAULT_CHILDREN_KEYS = [NestedFieldNames.Actions]; +const childrenMap = { + ['Microsoft.IfCondition']: [NestedFieldNames.Actions, NestedFieldNames.ElseActions], + ['Microsoft.SwitchCondition']: [NestedFieldNames.Cases, NestedFieldNames.DefaultCase], +}; + +/** + * Considering that an Adaptive Action could be nested with other actions, + * for example, the IfCondition and SwitchCondition and Foreach, we need + * this helper to visit all possible action nodes recursively. + * + * @param {any} input The input Adaptive Action which has $type field. + * @param {function} visitor The callback function called on each action node. + */ +export async function walkAdaptiveAction(input: any, visitor: (data: any) => Promise) { + if (!input || !input.$type) return; + + await visitor(input); + + let childrenKeys = DEFAULT_CHILDREN_KEYS; + if (input.$type && childrenMap[input.$type]) { + childrenKeys = childrenMap[input.$type]; + } + + for (const childrenKey of childrenKeys) { + const children = input[childrenKey]; + if (Array.isArray(children)) { + await Promise.all(children.map(async x => await walkAdaptiveAction(x, visitor))); + } + } +}