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

feat: Deep linking for the notification page #1667

Merged
merged 54 commits into from
Dec 7, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
729c2bf
deeplink for lg
lei9444 Nov 26, 2019
b956bd0
make lu link to dialog
lei9444 Nov 26, 2019
5728996
Merge branch 'master' into deeplink
lei9444 Nov 27, 2019
49e4940
make lu inline editor show error
lei9444 Nov 27, 2019
fd4fdb2
fix the lgeditor coderange error
lei9444 Nov 27, 2019
c58f800
fix code range type
lei9444 Nov 27, 2019
971376c
fix test
lei9444 Nov 27, 2019
f83a043
remove wait
lei9444 Nov 27, 2019
926ec36
edit e2e azure pipelines
lei9444 Nov 28, 2019
7252278
update e2e dir
lei9444 Nov 28, 2019
cedfece
fix some comments
lei9444 Nov 28, 2019
9ee13c1
Merge branch 'master' into deeplink
lei9444 Nov 29, 2019
27762f0
fix unit test
lei9444 Nov 29, 2019
678be71
Merge branch 'master' into deeplink
lei9444 Dec 2, 2019
216f243
add dialog check
lei9444 Dec 2, 2019
b86b979
add dialog error jump
lei9444 Dec 2, 2019
b0b72b3
fix lg templete type error
lei9444 Dec 2, 2019
4dadf1c
fix lint
lei9444 Dec 2, 2019
e291bcc
Merge branch 'master' into deeplink
lei9444 Dec 3, 2019
460b944
change the build order
lei9444 Dec 3, 2019
aba5ce6
fix conflict
lei9444 Dec 3, 2019
ae2ee21
fix some type
lei9444 Dec 3, 2019
b9b2ef1
disable start button if bot has error
lei9444 Dec 3, 2019
7136308
hide the emulater button
lei9444 Dec 3, 2019
4f4e4dc
fix e2e test and lg page crash
lei9444 Dec 3, 2019
4243ea5
add some e2e test and unit test
lei9444 Dec 3, 2019
2533c56
fix some comment
lei9444 Dec 4, 2019
f9f4d17
Merge branch 'master' into deeplink
lei9444 Dec 4, 2019
5174fae
move lgutil from shared to indexers
lei9444 Dec 4, 2019
afc3581
add e2e test
lei9444 Dec 4, 2019
6d90ee7
use indexer types
lei9444 Dec 4, 2019
aa90724
Merge branch 'master' into deeplink
lei9444 Dec 4, 2019
1d3d026
Merge branch 'master' into deeplink
lei9444 Dec 5, 2019
5a027ef
add schema to check expression field
lei9444 Dec 5, 2019
be2c128
make empty expression as waining
lei9444 Dec 5, 2019
9ebf451
fix lint and e2e test
lei9444 Dec 5, 2019
d1fbec1
fix test
lei9444 Dec 5, 2019
a264147
add some unit test
lei9444 Dec 5, 2019
7f8e233
fix a dialog nav bug
lei9444 Dec 5, 2019
940e37a
fix some ui comments
lei9444 Dec 6, 2019
940c85f
Merge branch 'master' into deeplink
lei9444 Dec 6, 2019
c65df3a
remove some code
lei9444 Dec 6, 2019
bac053f
fix unit test
lei9444 Dec 6, 2019
cd3c006
add empty check
lei9444 Dec 6, 2019
02c9f2f
make notifications single click
lei9444 Dec 6, 2019
73e51de
clean the tsconfig in shared
lei9444 Dec 6, 2019
0e4665a
fix list header
lei9444 Dec 6, 2019
cecc4c2
update the style
lei9444 Dec 6, 2019
f1b7319
update the e2e test
lei9444 Dec 6, 2019
cc9e72e
Merge branch 'master' into deeplink
lei9444 Dec 6, 2019
f583103
fix some conflict
lei9444 Dec 6, 2019
0b59aba
implement Pagination
lei9444 Dec 6, 2019
33a4386
fix pagination select from dropdown error
lei9444 Dec 6, 2019
7ad6124
fix expression function error
lei9444 Dec 7, 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
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,36 @@

import * as React from 'react';
import { render } from 'react-testing-library';
import { formatMessage } from 'format-message';

import { NotificationList } from '../../src/pages/notifications/NotificationList';

describe('<NotificationList/>', () => {
const items = [
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
{ type: 'Error', location: 'test1', message: 'error1' },
{ type: 'Warning', location: 'test2', message: 'error2' },
{ type: 'Error', location: 'test3', message: 'error3' },
{
id: 'Main.dialog',
severity: formatMessage('Error'),
type: 'dialog',
location: formatMessage('test1'),
message: formatMessage('error1'),
diagnostic: '',
},
{
id: 'Main.lu',
severity: formatMessage('Warning'),
type: 'lu',
location: formatMessage('test2'),
message: formatMessage('error2'),
diagnostic: '',
},
{
id: 'common.lg',
severity: formatMessage('Error'),
type: 'lg',
location: formatMessage('test3'),
message: formatMessage('error3'),
diagnostic: '',
},
];
it('should render the NotificationList', () => {
const { container } = render(<NotificationList items={items} />);
Expand Down
8 changes: 7 additions & 1 deletion Composer/packages/client/src/ShellApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import React, { useEffect, useContext, useMemo } from 'react';
import { ShellData } from '@bfc/shared';
import isEqual from 'lodash/isEqual';
import get from 'lodash/get';
import { LGTemplate } from 'botbuilder-lg';

import { isExpression } from './utils';
import * as lgUtil from './utils/lgUtil';
Expand Down Expand Up @@ -132,8 +133,13 @@ export const ShellApi: React.FC = () => {
if (id === undefined) throw new Error('must have a file id');
const file = lgFiles.find(file => file.id === id);
if (!file) throw new Error(`lg file ${id} not found`);
let templates: LGTemplate[] = [];
try {
templates = lgUtil.parse(file.content);
} catch (error) {
console.error(error);
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
}

const templates = lgUtil.parse(file.content);
const lines = file.content.split('\n');

return templates.map(t => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,19 @@ import get from 'lodash/get';
import debounce from 'lodash/debounce';
import isEmpty from 'lodash/isEmpty';
import { CodeRange, LgFile } from '@bfc/shared';
import { editor } from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api';

import * as lgUtil from '../../utils/lgUtil';

interface CodeEditorProps {
file: LgFile;
onChange: (value: string) => void;
codeRange?: Partial<CodeRange> | null;
codeRange?: Partial<CodeRange>;
onMount?: (editor: editor.IStandaloneCodeEditor) => void;
}

export default function CodeEditor(props: CodeEditorProps) {
const { file, codeRange } = props;
const { file, codeRange, onMount } = props;
const onChange = debounce(props.onChange, 500);
const [diagnostics, setDiagnostics] = useState(get(file, 'diagnostics', []));
const [content, setContent] = useState(get(file, 'content', ''));
Expand Down Expand Up @@ -53,7 +55,8 @@ export default function CodeEditor(props: CodeEditorProps) {
lineDecorationsWidth: undefined,
lineNumbersMinChars: false,
}}
codeRange={codeRange || -1}
codeRange={codeRange}
editorDidMount={onMount}
errorMsg={errorMsg}
value={content}
onChange={_onChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Nav, INavLinkGroup, INavLink } from 'office-ui-fabric-react/lib/Nav';
import get from 'lodash/get';
import { RouteComponentProps } from '@reach/router';
import { CodeRange } from '@bfc/shared';
import { editor } from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api';

import { LoadingSpinner } from '../../components/LoadingSpinner';
import { OpenAlertModal, DialogStyle } from '../../components/Modal';
Expand Down Expand Up @@ -37,12 +38,23 @@ const LGPage: React.FC<RouteComponentProps> = props => {
const { lgFiles, dialogs } = state;
const [editMode, setEditMode] = useState(false);
const [fileValid, setFileValid] = useState(true);
const [codeRange, setCodeRange] = useState<CodeRange | null>(null);
const [codeRange, setCodeRange] = useState<CodeRange>();
const [lgEditor, setLgEditor] = useState<editor.IStandaloneCodeEditor | null>(null);
lei9444 marked this conversation as resolved.
Show resolved Hide resolved

const hash = props.location ? props.location.hash : '';
const subPath = props['*'];
const isRoot = subPath === '';
const activeDialog = dialogs.find(item => item.id === subPath);

useEffect(() => {
if (hash && lgEditor) {
const match = /line=(\d+)/g.exec(hash);
if (match) {
lgEditor.revealLine(+match[1]);
}
}
}, [hash, lgEditor]);

// for now, one bot only have one lg file by default. all dialog share one lg
// file.
const lgFile = lgFiles.length ? lgFiles[0] : null;
Expand Down Expand Up @@ -96,7 +108,7 @@ const LGPage: React.FC<RouteComponentProps> = props => {

// fall back to the all-up page if we don't have an active dialog
if (!isRoot && !activeDialog && dialogs.length) {
navigateTo('/language-generation');
navigateTo('/language-generation/');
}
}, [subPath, dialogs]);

Expand All @@ -113,15 +125,15 @@ const LGPage: React.FC<RouteComponentProps> = props => {

function onSelect(id) {
if (id === '_all') {
navigateTo('/language-generation');
navigateTo('/language-generation/');
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
} else {
navigateTo(`language-generation/${id}`);
}
}

function onToggleEditMode() {
setEditMode(!editMode);
setCodeRange(null);
setCodeRange(undefined);
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
}

async function onChange(newContent) {
Expand Down Expand Up @@ -173,8 +185,9 @@ const LGPage: React.FC<RouteComponentProps> = props => {
css={actionButton}
onText={formatMessage('Edit mode')}
offText={formatMessage('Edit mode')}
defaultChecked={false}
checked={editMode}
disabled={(!isRoot && editMode === false) || (codeRange === null && fileValid === false)}
disabled={(!isRoot && editMode === false) || (!codeRange && fileValid === false)}
onChange={onToggleEditMode}
/>
</div>
Expand Down Expand Up @@ -215,7 +228,7 @@ const LGPage: React.FC<RouteComponentProps> = props => {
<div css={contentEditor}>
{editMode ? (
<Suspense fallback={<LoadingSpinner />}>
<CodeEditor file={lgFile} codeRange={codeRange} onChange={onChange} />
<CodeEditor file={lgFile} codeRange={codeRange} onChange={onChange} onMount={setLgEditor} />
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
</Suspense>
) : (
<TableView file={lgFile} activeDialog={activeDialog} onClickEdit={onTableViewClickEdit} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { notification, typeIcon, listRoot, icons } from './styles';

export interface INotificationListProps {
items: INotification[];
onItemInvoked: (item: INotification) => void;
}

const columns: IColumn[] = [
Expand All @@ -23,7 +24,8 @@ const columns: IColumn[] = [
minWidth: 30,
maxWidth: 30,
onRender: (item: INotification) => {
return <FontIcon iconName={icons[item.type].iconName} css={typeIcon(icons[item.type])} />;
const icon = icons[item.severity];
return <FontIcon iconName={icon.iconName} css={typeIcon(icon)} />;
},
},
{
Expand All @@ -36,7 +38,7 @@ const columns: IColumn[] = [
isResizable: true,
data: 'string',
onRender: (item: INotification) => {
return <span>{item.type}</span>;
return <span>{item.severity}</span>;
},
isPadded: true,
},
Expand Down Expand Up @@ -71,7 +73,7 @@ const columns: IColumn[] = [
];

export const NotificationList: React.FC<INotificationListProps> = props => {
const { items } = props;
const { items, onItemInvoked } = props;

return (
<div css={listRoot} data-testid="notifications-table-view">
Expand All @@ -82,6 +84,7 @@ export const NotificationList: React.FC<INotificationListProps> = props => {
setKey="none"
layoutMode={DetailsListLayoutMode.justified}
isHeaderVisible={true}
onItemInvoked={onItemInvoked}
/>
</div>
);
Expand Down
18 changes: 17 additions & 1 deletion Composer/packages/client/src/pages/notifications/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,31 @@ import useNotifications from './useNotifications';
import { NotificationList } from './NotificationList';
import { NotificationHeader } from './NotificationHeader';
import { root } from './styles';
import { INotification } from './types';
import { navigateTo } from './../../utils';

const navigations = {
lg: (item: INotification) => {
navigateTo(`/language-generation/#line=${item.diagnostic.Range.Start.Line || 0}`);
},
lu: (item: INotification) => {
navigateTo(`/dialogs/${item.id}`);
},
dialog: (item: INotification) => {
navigateTo(`/dialogs/${item.id}`);
},
};
const Notifications: React.FC<RouteComponentProps> = () => {
const [filter, setFilter] = useState('');
const { notifications, locations } = useNotifications(filter);
const handleItemInvoked = (item: INotification) => {
navigations[item.type](item);
};
return (
<div css={root} data-testid="notifications-page">
<ToolBar />
<NotificationHeader items={locations} onChange={setFilter} />
<NotificationList items={notifications} />
<NotificationList items={notifications} onItemInvoked={handleItemInvoked} />
</div>
);
};
Expand Down
3 changes: 3 additions & 0 deletions Composer/packages/client/src/pages/notifications/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
// Licensed under the MIT License.

export interface INotification {
id: string;
severity: string;
type: string;
location: string;
message: string;
diagnostic: any;
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,41 @@ export default function useNotifications(filter: string) {
dialog.diagnostics.map(diagnostic => {
const location = dialog.displayName;
locations.add(location);
notifactions.push({ type: 'Error', location, message: diagnostic });
notifactions.push({
type: 'dialog',
location,
message: diagnostic,
severity: 'Error',
diagnostic,
id: dialog.id,
});
});
});
luFiles.forEach(lufile => {
lufile.diagnostics.map(diagnostic => {
const location = `${lufile.id}.lu`;
locations.add(location);
notifactions.push({ type: 'Error', location, message: diagnostic.text });
notifactions.push({
type: 'lu',
location,
message: diagnostic.text,
severity: 'Error',
diagnostic,
id: lufile.id,
});
});
});
lgFiles.forEach(lgFiles => {
lgFiles.diagnostics.map(diagnostic => {
const location = `${lgFiles.id}.lg`;
lgFiles.forEach(lgFile => {
lgFile.diagnostics.map(diagnostic => {
const location = `${lgFile.id}.lg`;
locations.add(location);
notifactions.push({
type: DiagnosticSeverity[diagnostic.Severity],
type: 'lg',
severity: DiagnosticSeverity[diagnostic.Severity],
location,
message: createSingleMessage(diagnostic),
diagnostic,
id: lgFile.id,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import React, { useState, ReactElement, Suspense } from 'react';
import React, { useState, ReactElement, Suspense, useEffect } from 'react';
import formatMessage from 'format-message';
import { FieldProps } from '@bfcomposer/react-jsonschema-form';
import { Dropdown, ResponsiveMode, IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown';
Expand Down Expand Up @@ -35,6 +35,18 @@ export const RecognizerField: React.FC<FieldProps<MicrosoftIRecognizer>> = props
selectedFile && typeof props.formData === 'string' && props.formData.startsWith(selectedFile.id)
);

//make the inline editor show error message
useEffect(() => {
if (selectedFile && selectedFile.diagnostics.length > 0) {
const msg = selectedFile.diagnostics.reduce((msg: string, diagnostic) => {
return (msg += `${diagnostic.text}\n`);
}, '');
setErrorMsg(msg);
} else {
setErrorMsg('');
}
}, [selectedFile]);

const handleChange = (_, option?: IDropdownOption): void => {
if (option) {
switch (option.key) {
Expand Down Expand Up @@ -142,12 +154,7 @@ export const RecognizerField: React.FC<FieldProps<MicrosoftIRecognizer>> = props
{() => {
if (selectedFile && isLuFileSelected) {
const updateLuFile = (newValue?: string): void => {
shellApi
.updateLuFile({ id: selectedFile.id, content: newValue })
.then(() => setErrorMsg(''))
.catch(error => {
setErrorMsg(error);
});
shellApi.updateLuFile({ id: selectedFile.id, content: newValue }).catch(setErrorMsg);
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,14 @@ describe('lu operation', () => {
}
});

it('should throw error when lu content is invalid', async () => {
it('should update diagnostics when lu content is invalid', async () => {
const id = 'root';
const content = 'hello \n hello3';

await expect(proj.updateLuFile(id, content)).rejects.toThrow();
const luFiles = await proj.updateLuFile(id, content);
const result = luFiles.find(f => f.id === id);
if (result !== undefined) {
await expect(result.diagnostics.length).toBeGreaterThan(0);
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
}
});

it('should delete lu file and update index', async () => {
Expand Down
11 changes: 0 additions & 11 deletions Composer/packages/server/src/models/bot/botProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import fs from 'fs';

import isEqual from 'lodash/isEqual';
import { FileInfo, DialogInfo, LgFile, LuFile, getNewDesigner } from '@bfc/shared';
import { dialogIndexer, luIndexer, lgIndexer } from '@bfc/indexers';

Expand Down Expand Up @@ -267,16 +266,6 @@ export class BotProject {
if (luFile === undefined) {
throw new Error(`no such lu file ${id}`);
}
let currentLufileParsedContentLUISJsonStructure = null;
try {
currentLufileParsedContentLUISJsonStructure = await luIndexer.parse(content);
} catch (error) {
throw new Error(`Update ${id}.lu Failed, ${error.text}`);
}

const preLufileParsedContentLUISJsonStructure = luFile.parsedContent.LUISJsonStructure;
const isUpdate = !isEqual(currentLufileParsedContentLUISJsonStructure, preLufileParsedContentLUISJsonStructure);
if (!isUpdate) return this.luFiles;

await this._updateFile(luFile.relativePath, content);
await this.luPublisher.onFileChange(luFile.relativePath, FileUpdateType.UPDATE);
Expand Down