Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
28c7599
implement 'deepCopyAction' for copying action recursively
yeze322 Oct 11, 2019
972e553
implement async lg template copy
yeze322 Oct 11, 2019
2217d46
inject lgApi into jsonTracker copy / cut
yeze322 Oct 11, 2019
a47d52e
add mock lgApi in demo
yeze322 Oct 11, 2019
3eebc55
make LgApi promise work
yeze322 Oct 11, 2019
a16d7d1
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 11, 2019
e6e5268
fix two unstable CI spec (why it started to failed here? it should ha…
yeze322 Oct 12, 2019
f880cf2
Merge branch 'zeye/copy-activity' of https://github.com/Microsoft/Bot…
yeze322 Oct 12, 2019
301b850
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 14, 2019
4372400
increase Breadcrumb spec waiting time to stablize
yeze322 Oct 14, 2019
5b9a245
Merge branch 'zeye/copy-activity' of https://github.com/Microsoft/Bot…
yeze322 Oct 14, 2019
20deb95
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 14, 2019
0ab66e0
fix CI by adding wait()
yeze322 Oct 14, 2019
78393e6
Merge branch 'zeye/copy-activity' of https://github.com/Microsoft/Bot…
yeze322 Oct 14, 2019
9b7075d
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 14, 2019
3b5a8f1
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 15, 2019
acba6eb
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 15, 2019
c6c7a91
remove comments
yeze322 Oct 15, 2019
84c954b
visit -> walk
yeze322 Oct 15, 2019
f2d7417
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 15, 2019
0d25c59
apply ''needsDeepCopy"
yeze322 Oct 15, 2019
29ab0c4
Fix lint error
yeze322 Oct 15, 2019
a628c69
Merge branch 'zeye/copy-activity' of https://github.com/Microsoft/Bot…
yeze322 Oct 15, 2019
78815ac
Merge branch 'master' of https://github.com/Microsoft/BotFramework-Co…
yeze322 Oct 16, 2019
500be04
move all copy logic into copyUtils
yeze322 Oct 16, 2019
594d1ed
use a function map to handle different types overrider
yeze322 Oct 17, 2019
b92aebc
simplify the overrider map
yeze322 Oct 17, 2019
72fbe49
remove needsDeepCopy handler outside
yeze322 Oct 17, 2019
9ed7aa6
judge needsOverride
yeze322 Oct 17, 2019
026000b
delete prefix for coped node by adding new lg template for copied node
alanlong9278 Oct 17, 2019
9335d44
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 17, 2019
dda7fb1
revert changes in copyUtils
yeze322 Oct 17, 2019
e369844
Merge branch 'zeye/copy-activity--local' into zeye/copy-activity
yeze322 Oct 17, 2019
35adcdf
remove unnecessary export
yeze322 Oct 17, 2019
4b368db
deep copy node at 'paste' stage instead of 'copy'
yeze322 Oct 17, 2019
7f8b749
create new lg at 'paste' stage
yeze322 Oct 17, 2019
96788fe
Merge branch 'master' into zeye/copy-activity
alanlong9278 Oct 18, 2019
fbb74ee
Merge branch 'master' into zeye/copy-activity
boydc2014 Oct 18, 2019
df52ffa
eslint
alanlong9278 Oct 18, 2019
8166737
Merge branch 'zeye/copy-activity' of https://github.com/microsoft/Bot…
alanlong9278 Oct 18, 2019
b3174b4
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 18, 2019
e6a2130
add unit tests for copyUtils
yeze322 Oct 18, 2019
f420982
Merge branch 'zeye/copy-activity' of https://github.com/Microsoft/Bot…
yeze322 Oct 18, 2019
039d255
Merge branch 'master' into zeye/copy-activity
yeze322 Oct 18, 2019
c0b48d5
Merge branch 'master' into zeye/copy-activity
boydc2014 Oct 18, 2019
3d571d2
Merge branch 'master' into zeye/copy-activity
cwhitten Oct 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Composer/cypress/integration/Breadcrumb.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ context('breadcrumb', () => {
before(() => {
cy.visit(Cypress.env('COMPOSER_URL'));
cy.openBot('ToDoBot');
cy.wait(100);
});

beforeEach(() => {
// Return to Main.dialog
cy.get('[data-testid="ProjectTree"]').within(() => {
cy.wait(1000);
cy.getByText('ToDoBot.Main').click();
cy.wait(1000);
});
});

Expand All @@ -30,6 +33,7 @@ context('breadcrumb', () => {
// Return to Main.dialog
cy.get('[data-testid="ProjectTree"]').within(() => {
cy.getByText('ToDoBot.Main').click();
cy.wait(100);
});

cy.getByTestId('Breadcrumb')
Expand Down
14 changes: 14 additions & 0 deletions Composer/cypress/integration/VisualDesigner.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,23 @@ context('Visual Designer', () => {
before(() => {
cy.visit(Cypress.env('COMPOSER_URL'));
cy.openBot('ToDoBot');
cy.wait(100);
});

beforeEach(() => {
// Return to Main.dialog
cy.get('[data-testid="ProjectTree"]').within(() => {
cy.getByText('ToDoBot.Main').click();
cy.wait(100);
});
});

it('can find Visual Designer default trigger in container', () => {
cy.get('[data-testid="ProjectTree"]').within(() => {
cy.getByText('Handle ConversationUpdate').click();
cy.wait(500);
});

cy.withinEditor('VisualEditor', () => {
cy.getByText('Trigger').should('exist');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ export class VisualEditorDemo extends Component {
obiJson: json,
});
},
getLgTemplates: () => {
return Promise.resolve([{ Name: 'lg', Body: 'LgTemplate Placeholder.' }]);
},
removeLgTemplate: () => {
return Promise.resolve(true);
},
}}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,13 @@ export const ObiEditor: FC<ObiEditorProps> = ({
}): JSX.Element | null => {
let divRef;

const { focusedId, focusedEvent, removeLgTemplate } = useContext(NodeRendererContext);
const { focusedId, focusedEvent, getLgTemplates, removeLgTemplate } = useContext(NodeRendererContext);
const [clipboardContext, setClipboardContext] = useState({
clipboardActions: [],
setClipboardActions: actions => setClipboardContext({ ...clipboardContext, clipboardActions: actions }),
});

const lgApi = { getLgTemplates, removeLgTemplate };
const dispatchEvent = (eventName: NodeEventTypes, eventData: any): any => {
let handler;
switch (eventName) {
Expand Down Expand Up @@ -98,16 +99,18 @@ export const ObiEditor: FC<ObiEditorProps> = ({
break;
case NodeEventTypes.CopySelection:
handler = e => {
const copiedActions = copyNodes(data, e.actionIds);
clipboardContext.setClipboardActions(copiedActions);
copyNodes(data, e.actionIds, lgApi).then(copiedActions => {
clipboardContext.setClipboardActions(copiedActions);
});
};
break;
case NodeEventTypes.CutSelection:
handler = e => {
const { dialog, cutData } = cutNodes(data, e.actionIds);
clipboardContext.setClipboardActions(cutData);
onChange(dialog);
onFocusSteps([]);
cutNodes(data, e.actionIds, lgApi).then(({ dialog, cutData }) => {
clipboardContext.setClipboardActions(cutData);
onChange(dialog);
onFocusSteps([]);
});
};
break;
case NodeEventTypes.DeleteSelection:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { cloneDeep, get, set } from 'lodash';
import { seedNewDialog } from 'shared';
import { seedNewDialog, deepCopyAction } from 'shared';

import { getFriendlyName } from '../components/nodes/utils';

Expand Down Expand Up @@ -168,13 +168,19 @@ export function insert(inputDialog, path, position, $type) {
return dialog;
}

export function copyNodes(inputDialog, nodeIds: string[]): any[] {
export async function copyNodes(inputDialog, nodeIds: string[], lgApi): Promise<any[]> {
const nodes = nodeIds.map(id => queryNode(inputDialog, id)).filter(x => x !== null);
return JSON.parse(JSON.stringify(nodes));
const copiedNodes = await Promise.all(
nodes.map(async x => {
const node = await deepCopyAction(x, lgApi);
Comment thread
yeze322 marked this conversation as resolved.
Outdated
return node;
})
);
return copiedNodes;
}

export function cutNodes(inputDialog, nodeIds: string[]) {
const nodesData = copyNodes(inputDialog, nodeIds);
export async function cutNodes(inputDialog, nodeIds: string[], lgApi) {
const nodesData = await copyNodes(inputDialog, nodeIds, lgApi);
const newDialog = deleteNodes(inputDialog, nodeIds);

return { dialog: newDialog, cutData: nodesData };
Expand Down
62 changes: 62 additions & 0 deletions Composer/packages/lib/shared/src/copyUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
export const NestedFieldNames = {
Actions: 'actions',
ElseActions: 'elseActions',
DefaultCase: 'default',
Cases: 'cases',
};

const DEFAULT_CHILDREN_KEYS = [NestedFieldNames.Actions];
const childrenMap = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those 3 properties are inventing a new abstraction which i'm not sure it's necessary, and make this part less readable.

Can you refer to "jsonWalker" in server package, which is generic json walking implementation, and the concept of "json walk" is already a well-known concept, we don't have re-invent one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is it? do we have plan on how can we share things in shared with server? I tend to no to do that here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point was "JsonWalk" is a well-solved problem, and has many implementation out there, and we also have one in server package. We don't have to choose the one in server, the point is implementing a customized xxWalk and not based on a generic JsonWalk looks like we are reinventing something, and making code less readable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my mind, json walk is a simple problem could be solved by DFS, do we have any reference link?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

google walk-json, json walk npm, or any keyword.

JsonWalk is a simple problem, can be achieved by DFS or other approach. The point is if we do some walk on json. A good pattern is use a generic jsonWalk which you trusted and tested seperately, plus your sepecific interested type of node. It's the classic "visitor pattern".

So "walkAdaptiveJson" can be done as "walkJson + adapativeVisitor". your walkJson method can be tested and verifiy with 100% coverage, your adpativeVisitor can be tested with 100% coverage. and they are both relative small, relaitvely focused. You don't care about specific node type when you are testing "jsonWalk",you don't care "DFS" when you testing "visitors".
it would be also result in a more readable design.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is how the code will looks like

function visitor(path, value) {
if (value.$type && value.$type == "SendActivity") {
value.activity = // create new activity
}
}

function copyNode(data) {
const newData = JSON.parse(...) // copy
jsonWalk('$', newData, visitor); // walk on new data
}

it can be simplified so much.

['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 visitAdaptiveAction(input: any, visitor: (data: any) => Promise<any>) {
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)) {
children.forEach(x => visitAdaptiveAction(x, visitor));
}
}
}

function isLgActivity(activity: string) {
return activity && activity.indexOf('bfdactivity-') !== -1;
}

export async function copyLgActivity(activity: string, lgApi: any): Promise<string> {
if (!activity) return '';
if (!isLgActivity(activity) || !lgApi) return activity;

const { getLgTemplates } = 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) return currentLg.Body;
return activity;
}
Comment thread
yeze322 marked this conversation as resolved.
30 changes: 27 additions & 3 deletions Composer/packages/lib/shared/src/dialogFactory.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import nanoid from 'nanoid/generate';

import { appschema } from './appschema';
import { visitAdaptiveAction, copyLgActivity } from './copyUtils';

interface DesignerAttributes {
name: string;
Expand Down Expand Up @@ -43,8 +44,8 @@ export function getNewDesigner(name: string, description: string) {

export const getDesignerId = (data?: DesignerData) => {
const newDesigner: DesignerData = {
id: nanoid('1234567890', 6),
...data,
id: nanoid('1234567890', 6),
};

return newDesigner;
Expand Down Expand Up @@ -77,14 +78,37 @@ export const needsDeepCopy = $type => {
return DEEP_COPY_TYPES.includes($type);
};

export const deepCopy: any = (_data, _lgApi) => {
export const deepCopyAction = async (data, lgApi) => {
// data.type is a SendActivity
// data.id is bound to copied SendActivity
// new id getDesignerId()
// data.activity references an LG template
// make new LG template based off of naming schema
// assign to data.activity
return undefined;
if (!data || !data.$type) return {};
Comment thread
yeze322 marked this conversation as resolved.
Outdated

// Deep copy the original data.
const copy = JSON.parse(JSON.stringify(data));

// Copy the $designer part (if exists) or create new $designer part.
const overrideDesigner = data => {
const $designer = data.$designer ? getDesignerId(data.$designer) : getNewDesigner('', '');
data.$designer = $designer;
};

// Copy specific parts an Adaptive action cares.
const overrideContent = async data => {
if (data.$type === 'Microsoft.SendActivity') {
data.activity = await copyLgActivity(data.activity, lgApi);
}
};

await visitAdaptiveAction(copy, async data => {
Comment thread
yeze322 marked this conversation as resolved.
Outdated
overrideDesigner(data);
await overrideContent(data);
});

return copy;
};

export const seedNewDialog = (
Expand Down