This repository was archived by the owner on Jul 9, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 375
[WIP] LG activity copy behavior: copies all possible lg templates #1193
Closed
Closed
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
f080966
copy other lg fields
yeze322 be0202c
optmize
yeze322 88af3c7
Merge branch 'master' into zeye/fix-lg-copy
boydc2014 4b0b084
extract lgUtils out of copyUtils
yeze322 2f90434
update lg template regex
yeze322 ccb4763
refine copyLgActivity API
yeze322 fbb44ca
add type for copyLgTemplate
yeze322 92b2fed
move lgUtils to Shell
yeze322 9eb46a5
fix lgUtils return value format
yeze322 e12fcd6
rename input ap name to copyLgTemplate
yeze322 b9ce7b3
fix lgUtils test
yeze322 9bc39d0
pass down copyLgTemplate from Shell instead of in shared lib
yeze322 67e2dbd
rename lgUtils -> lgHandlers
yeze322 5f72af0
fix the format of newTemplateName
yeze322 a0a3733
change test name of lgHandlers
yeze322 237cb0c
group imports in ExtensionContainer
yeze322 79e470d
Merge branch 'master' into zeye/fix-lg-copy
yeze322 eabf7ce
update several ts types
yeze322 b2497a5
use lodash.cloneDeep in copyUtils
yeze322 237f6a1
move walker to another file
yeze322 bb62953
move copyUtils and walk to 'utils' folder
yeze322 e279a71
move copyUtils.test to sub folder
yeze322 d5094f0
add UT for 'walkAdaptiveAction'
yeze322 ce00b7b
fix async version walk + UT
yeze322 9462253
Merge branch 'master' into zeye/fix-lg-copy
boydc2014 91658bf
Merge branch 'master' into zeye/fix-lg-copy
boydc2014 2e2f050
Merge branch 'master' into zeye/fix-lg-copy
boydc2014 7ceec4e
Merge branch 'master' into zeye/fix-lg-copy
yeze322 db2016a
Merge branch 'master' into zeye/fix-lg-copy
boydc2014 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
71 changes: 71 additions & 0 deletions
71
Composer/packages/client/__tests__/store/action/lgHandlers.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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'); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| const TEMPLATE_PATTERN = /^\[(bfd.+-\d+)\]$/; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file does not belong with the other actions. |
||
| 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<any>; | ||
| updateLgTemplate: (lgFileName: string, newTemplateName: string, newContent: string) => Promise<any>; | ||
| } | ||
| ): Promise<string> { | ||
| if (!templateNameToCopy) return ''; | ||
| if (!isLgActivity(templateNameToCopy) || !lgApi) return templateNameToCopy; | ||
|
|
||
| const { getLgTemplates, updateLgTemplate } = lgApi; | ||
| if (!getLgTemplates) return templateNameToCopy; | ||
|
|
||
| let rawLg: any[] = []; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be typed? |
||
| try { | ||
| rawLg = await getLgTemplates(lgFileName, templateNameToCopy); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We really need to add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes! |
||
| } 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; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
Composer/packages/lib/shared/__tests__/utils/walkAdaptiveAction.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be an api call? Also, you are importing
copyLgTemplatefrom the actions, but it does not conform to the action interface. This looks like a code smell to me.Maybe
copyLgTemplateis a shared utility, not an action? Also, passing in thegetandupdatefunctions seem like a smell. Why is this dependency injection needed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with you,
copyLgTemplateshould be an API but it's not provided by server. So I simulated an API here.