Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Delete & Undo on actions with LG templates doesn't bring back the LG content #4740

Merged
merged 11 commits into from
Nov 10, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ export function useDialogEditApi(shellApi: ShellApi) {
}

function deleteSelectedActions(dialogId: string, dialogData, actionIds: string[]) {
return deleteNodes(dialogData, actionIds, (nodes) => {
deleteActions(dialogId, nodes);
});
return deleteNodes(dialogData, actionIds, (nodes) => deleteActions(dialogId, nodes));
}

function disableSelectedActions(dialogId: string, dialogData, actionIds: string[]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,12 @@ export const useEditorEventApi = (
case NodeEventTypes.Delete:
trackActionChange(eventData.id);
handler = (e) => {
onChange(deleteSelectedAction(path, data, e.id), undefined, async () => {
await onFocusSteps([]);
announce(ScreenReaderMessage.ActionDeleted);
});
deleteSelectedAction(path, data, e.id).then((value) =>
onChange(value, undefined, async () => {
await onFocusSteps([]);
announce(ScreenReaderMessage.ActionDeleted);
})
);
};
break;
case NodeEventTypes.Insert:
Expand Down Expand Up @@ -272,10 +274,12 @@ export const useEditorEventApi = (
handler = () => {
const actionIds = getClipboardTargetsFromContext();
trackActionListChange(actionIds);
onChange(deleteSelectedActions(path, data, actionIds), undefined, async () => {
await onFocusSteps([]);
announce(ScreenReaderMessage.ActionsDeleted);
});
deleteSelectedActions(path, data, actionIds).then((value) =>
onChange(value, undefined, async () => {
await onFocusSteps([]);
announce(ScreenReaderMessage.ActionsDeleted);
})
);
};
break;
case NodeEventTypes.DisableSelection:
Expand Down
2 changes: 1 addition & 1 deletion Composer/packages/client/src/shell/lgApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function createLgApi(
addLgTemplate: updateLgTemplate,
getLgTemplates,
updateLgTemplate,
debouncedUpdateLgTemplate: debounce(updateLgTemplate, 250),
debouncedUpdateLgTemplate: debounce(updateLgTemplate, 250, { leading: true, trailing: false }),
removeLgTemplate,
removeLgTemplates,
copyLgTemplate,
Expand Down
2 changes: 1 addition & 1 deletion Composer/packages/client/src/shell/luApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ function createLuApi(
getLuIntents,
getLuIntent,
updateLuIntent,
debouncedUpdateLuIntent: debounce(updateLuIntent, 250),
debouncedUpdateLuIntent: debounce(updateLuIntent, 250, { leading: true, trailing: false }),
renameLuIntent,
removeLuIntent,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,34 +38,34 @@ describe('delete node flow', () => {
});

describe('when target node does not exist', () => {
it('should not change the data', () => {
it('should not change the data', async () => {
path = null;
const result = deleteNode(dialog, path, removedDataFn);
const result = await deleteNode(dialog, path, removedDataFn);

expect(result).toEqual(dialog);
expect(removedDataFn).not.toBeCalled();
});
});

describe('when target node exists', () => {
it("should delete node successfully when targetNode's currentKey type is number", () => {
it("should delete node successfully when targetNode's currentKey type is number", async () => {
path = 'foo.bar[0]';
const result = deleteNode(dialog, path, removedDataFn);
const result = await deleteNode(dialog, path, removedDataFn);

expect(result).toEqual({ foo: { bar: [{ $kind: 'secondOne' }] } });
expect(removedDataFn).toBeCalledWith(dialog.foo.bar[0]);
});
it("should delete node successfully when targetNode's currentKey type is string", () => {
it("should delete node successfully when targetNode's currentKey type is string", async () => {
path = 'foo.bar';
const result = deleteNode(dialog, path, removedDataFn);
const result = await deleteNode(dialog, path, removedDataFn);

expect(result).toEqual({ foo: {} });
expect(removedDataFn).toBeCalledWith(dialog.foo.bar);
});
it("removeLgTemplate function should be called when targetNode's $kind is 'Microsoft.SendActivity' && activity includes '[SendActivity_'", () => {
it("removeLgTemplate function should be called when targetNode's $kind is 'Microsoft.SendActivity' && activity includes '[SendActivity_'", async () => {
dialog.foo.activityNode = { $kind: 'Microsoft.SendActivity', activity: '[SendActivity_a]' };
path = 'foo.activityNode';
const result = deleteNode(dialog, path, removedDataFn);
const result = await deleteNode(dialog, path, removedDataFn);

expect(removedDataFn).toBeCalledWith(dialog.foo.activityNode);
expect(result).toEqual({ foo: { bar: [{ $kind: 'firstOne' }, { $kind: 'secondOne' }] } });
Expand Down
20 changes: 10 additions & 10 deletions Composer/packages/lib/shared/src/deleteUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,32 +48,32 @@ const collectLuIntents = (action: any, outputTemplates: string[]) => {
}
};

export const deleteAdaptiveAction = (
export const deleteAdaptiveAction = async (
data: MicrosoftIDialog,
deleteLgTemplates: (lgTemplates: string[]) => any,
deleteLuIntents: (luIntents: string[]) => any
deleteLgTemplates: (lgTemplates: string[]) => Promise<void>,
deleteLuIntents: (luIntents: string[]) => Promise<void>
) => {
const lgTemplates: string[] = [];
const luIntents: string[] = [];

walkAdaptiveAction(data, (action) => collectLgTemplates(action, lgTemplates));
walkAdaptiveAction(data, (action) => collectLuIntents(action, luIntents));

deleteLgTemplates(lgTemplates.filter((activity) => !!activity));
deleteLuIntents(luIntents);
await deleteLgTemplates(lgTemplates.filter((activity) => !!activity));
await deleteLuIntents(luIntents);
};

export const deleteAdaptiveActionList = (
export const deleteAdaptiveActionList = async (
data: MicrosoftIDialog[],
deleteLgTemplates: (lgTemplates: string[]) => any,
deleteLuIntents: (luIntents: string[]) => any
deleteLgTemplates: (lgTemplates: string[]) => Promise<void>,
deleteLuIntents: (luIntents: string[]) => Promise<void>
) => {
const lgTemplates: string[] = [];
const luIntents: string[] = [];

walkAdaptiveActionList(data, (action) => collectLgTemplates(action, lgTemplates));
walkAdaptiveActionList(data, (action) => collectLuIntents(action, luIntents));

deleteLgTemplates(lgTemplates.filter((activity) => !!activity));
deleteLuIntents(luIntents);
await deleteLgTemplates(lgTemplates.filter((activity) => !!activity));
await deleteLuIntents(luIntents);
};
8 changes: 4 additions & 4 deletions Composer/packages/lib/shared/src/dialogFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,16 +356,16 @@ export const deepCopyActions = async (

export const deleteAction = (
data: MicrosoftIDialog,
deleteLgTemplates: (templates: string[]) => any,
deleteLuIntents: (luIntents: string[]) => any
deleteLgTemplates: (templates: string[]) => Promise<any>,
deleteLuIntents: (luIntents: string[]) => Promise<any>
) => {
return deleteAdaptiveAction(data, deleteLgTemplates, deleteLuIntents);
};

export const deleteActions = (
inputs: MicrosoftIDialog[],
deleteLgTemplates: (templates: string[]) => any,
deleteLuIntents: (luIntents: string[]) => any
deleteLgTemplates: (templates: string[]) => Promise<any>,
deleteLuIntents: (luIntents: string[]) => Promise<any>
) => {
return deleteAdaptiveActionList(inputs, deleteLgTemplates, deleteLuIntents);
};
Expand Down
12 changes: 8 additions & 4 deletions Composer/packages/lib/shared/src/dialogUtils/jsonTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function queryNodes(inputDialog, nodeIds: string[]) {
return nodeIds.map((id) => queryNode(inputDialog, id)).filter((x) => x !== null);
}

export function deleteNode(inputDialog, path, callbackOnRemovedData?: (removedData: any) => any) {
export async function deleteNode(inputDialog, path, callbackOnRemovedData?: (removedData: any) => Promise<any>) {
const dialog = cloneDeep(inputDialog);
const target = locateNode(dialog, path);
if (!target) return dialog;
Expand All @@ -117,13 +117,17 @@ export function deleteNode(inputDialog, path, callbackOnRemovedData?: (removedDa

// invoke callback handler
if (callbackOnRemovedData && typeof callbackOnRemovedData === 'function') {
callbackOnRemovedData(deletedData);
await callbackOnRemovedData(deletedData);
}

return dialog;
}

export function deleteNodes(inputDialog, nodeIds: string[], callbackOnRemovedNodes?: (nodes: any[]) => any) {
export async function deleteNodes(
inputDialog,
nodeIds: string[],
callbackOnRemovedNodes?: (nodes: any[]) => Promise<any>
) {
const dialog = cloneDeep(inputDialog);

const nodeLocations = nodeIds.map((id) => locateNode(dialog, id));
Expand Down Expand Up @@ -151,7 +155,7 @@ export function deleteNodes(inputDialog, nodeIds: string[], callbackOnRemovedNod

// invoke callback handler
if (callbackOnRemovedNodes && typeof callbackOnRemovedNodes === 'function') {
callbackOnRemovedNodes(deletedNodes);
await callbackOnRemovedNodes(deletedNodes);
}

return dialog;
Expand Down
2 changes: 1 addition & 1 deletion Composer/packages/types/src/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export type LuContextApi = {
updateLuIntent: (id: string, intentName: string, intent: LuIntentSection) => Promise<LuFile[] | undefined>;
debouncedUpdateLuIntent: (id: string, intentName: string, intent: LuIntentSection) => Promise<LuFile[] | undefined>;
renameLuIntent: (id: string, intentName: string, newIntentName: string) => Promise<LuFile[] | undefined>;
removeLuIntent: (id: string, intentName: string) => void;
removeLuIntent: (id: string, intentName: string) => Promise<LuFile[] | undefined>;
};

export type LgContextApi = {
Expand Down
16 changes: 10 additions & 6 deletions Composer/packages/ui-plugins/lg/src/LgField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ const LgField: React.FC<FieldProps<string>> = (props) => {
const lgFile = lgFiles && lgFiles.find((file) => file.id === lgFileId);

const updateLgTemplate = useCallback(
(body: string) => {
shellApi.debouncedUpdateLgTemplate(lgFileId, lgName, body);
async (body: string) => {
await shellApi.debouncedUpdateLgTemplate(lgFileId, lgName, body);
},
[lgName, lgFileId]
);
Expand All @@ -76,12 +76,16 @@ const LgField: React.FC<FieldProps<string>> = (props) => {
const onChange = (body: string) => {
if (designerId) {
if (body) {
updateLgTemplate(body);
updateLgTemplate(body).then(() => {
if (lgTemplateRef) {
shellApi.commitChanges();
}
});
props.onChange(new LgTemplateRef(lgName).toString());
shellApi.commitChanges();
} else {
shellApi.removeLgTemplate(lgFileId, lgName);
props.onChange();
shellApi.removeLgTemplate(lgFileId, lgName).then(() => {
props.onChange();
});
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion Composer/packages/ui-plugins/luis/src/LuisIntentEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const LuisIntentEditor: React.FC<FieldProps<string>> = (props) => {
}

const newIntent = { Name: intentName, Body: newValue };
shellApi.debouncedUpdateLuIntent(luFile.id, intentName, newIntent);
shellApi.debouncedUpdateLuIntent(luFile.id, intentName, newIntent)?.then(shellApi.commitChanges);
onChange(intentName);
};

Expand Down