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: botprojects context menu issues #4866

Merged
merged 20 commits into from
Nov 22, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e478a84
when check manifest filter root bot
zhixzhan Nov 17, 2020
0c4e7d1
do not truncat on tooltips
zhixzhan Nov 18, 2020
7be58d1
fix create/edit manifest on projectTree
zhixzhan Nov 18, 2020
2dbd26b
Merge branch 'feature/bot-projects' into zhixzhan/bot-projects-fix
zhixzhan Nov 18, 2020
ec60a69
validate skill from .botproj
zhixzhan Nov 18, 2020
45e682f
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
srinaath Nov 18, 2020
784317a
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
zhixzhan Nov 19, 2020
d22562d
lint
zhixzhan Nov 19, 2020
323ef47
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
zhixzhan Nov 19, 2020
5c34908
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
zhixzhan Nov 20, 2020
50bed32
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
zhixzhan Nov 20, 2020
9855406
handle error click in context menu
lei9444 Nov 21, 2020
cddba71
remove useless import
lei9444 Nov 21, 2020
6fc7f10
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
srinaath Nov 21, 2020
ff0a5a2
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
lei9444 Nov 21, 2020
283ff15
Merge branch 'feature/bot-projects' of https://github.com/microsoft/B…
lei9444 Nov 21, 2020
50587de
fix conflict
lei9444 Nov 21, 2020
5390124
Merge branch 'feature/bot-projects' into zhixzhan/feature/bot-project…
cwhitten Nov 21, 2020
5177754
use callout to repalce tooltip
lei9444 Nov 21, 2020
d348a66
close callout after click the link
lei9444 Nov 21, 2020
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 @@ -16,7 +16,7 @@ import { IDialogContentStyles } from 'office-ui-fabric-react/lib/Dialog';
import { IModalStyles } from 'office-ui-fabric-react/lib/Modal';
import { useRecoilValue } from 'recoil';

import { skillsStateSelector, userSettingsState } from '../../recoilModel';
import { skillNameIdentifierByProjectIdSelector, skillsStateSelector, userSettingsState } from '../../recoilModel';

// -------------------- Styles -------------------- //

Expand Down Expand Up @@ -61,19 +61,20 @@ interface DisplayManifestModalProps {
export const DisplayManifestModal: React.FC<DisplayManifestModalProps> = ({
isDraggable = true,
isModeless = true,
skillNameIdentifier,
projectId,
onDismiss,
}) => {
const skills = useRecoilValue(skillsStateSelector);
const userSettings = useRecoilValue(userSettingsState);

const skillsByProjectId = useRecoilValue(skillNameIdentifierByProjectIdSelector);
const skillNameIdentifier = skillsByProjectId[projectId];
useEffect(() => onDismiss, []);

const selectedSkill = useMemo(() => {
if (skillNameIdentifier) {
return skills[skillNameIdentifier];
}
}, [skillNameIdentifier]);
}, [skillNameIdentifier, skills]);

if (!selectedSkill) {
return null;
Expand Down
42 changes: 21 additions & 21 deletions Composer/packages/client/src/components/ProjectTree/treeItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { IButtonStyles } from 'office-ui-fabric-react/lib/Button';
import { IContextualMenuStyles } from 'office-ui-fabric-react/lib/ContextualMenu';
import { ICalloutContentStyles } from 'office-ui-fabric-react/lib/Callout';
import { DiagnosticSeverity, Diagnostic } from '@bfc/shared';
import isEmpty from 'lodash/isEmpty';

import { createBotSettingUrl, navigateTo } from '../../utils/navigation';

Expand Down Expand Up @@ -132,19 +133,13 @@ const navItem = (

export const diagnosticLink = css`
display: flex;
align-items: center;
span {
align-content: start;
p {
margin: 2px 5px;
width: 300px;
}
`;

export const diagnosticLinkMessages = css`
max-width: 200px;
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;
`;

export const overflowSet = (isBroken: boolean) => css`
width: 100%;
height: 100%;
Expand Down Expand Up @@ -265,10 +260,10 @@ const onRenderItem = (textWidth: number, showErrors: boolean) => (item: IOverflo
return (
<div key={item.message} css={diagnosticLink}>
<Icon iconName={'Warning'} style={diagnosticWarningIcon} />
<span css={diagnosticLinkMessages} title={item.message}>
<p title={item.message}>
{item.message}
</span>
<Link>{linkText}</Link>
<Link styles={{ root: { marginLeft: '5px' } }}>{linkText}</Link>
</p>
</div>
);
});
Expand All @@ -281,26 +276,31 @@ const onRenderItem = (textWidth: number, showErrors: boolean) => (item: IOverflo
return (
<div key={item.message} css={diagnosticLink}>
<Icon iconName={'ErrorBadge'} style={diagnosticErrorIcon} />
<span css={diagnosticLinkMessages} title={item.message}>
<p title={item.message}>
{item.message}
</span>
<Link onClick={() => navigateTo(createBotSettingUrl(projectId, skillId ?? projectId))}>{linkText}</Link>
<Link
styles={{ root: { marginLeft: '5px' } }}
onClick={() => navigateTo(createBotSettingUrl(projectId, skillId ?? projectId))}
>
{linkText}
</Link>
</p>
</div>
);
});

diagnosticIcons = (
<React.Fragment>
{warnings.length ? (
<TooltipHost content={warningHTML} directionalHint={DirectionalHint.bottomLeftEdge}>
{!isEmpty(warnings) && (
<TooltipHost closeDelay={1000} content={warningHTML} directionalHint={DirectionalHint.bottomLeftEdge}>
<Icon iconName={'WarningSolid'} style={warningIcon} />
</TooltipHost>
) : undefined}
{errors.length ? (
<TooltipHost content={errorHTML} directionalHint={DirectionalHint.bottomLeftEdge}>
)}
{!isEmpty(errors) && (
<TooltipHost closeDelay={1000} content={errorHTML} directionalHint={DirectionalHint.bottomLeftEdge}>
<Icon iconName={'StatusErrorFull'} style={errorIcon} />
</TooltipHost>
) : undefined}
)}
</React.Fragment>
);
}
Expand Down
13 changes: 6 additions & 7 deletions Composer/packages/client/src/pages/design/DesignPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
userSettingsState,
dispatcherState,
schemasState,
displaySkillManifestState,
validateDialogsSelectorFamily,
focusPathState,
showCreateDialogModalState,
Expand All @@ -46,6 +45,7 @@ import {
skillNameIdentifierByProjectIdSelector,
SkillInfo,
projectMetaDataState,
displayManifestModalOnProjectIdSelector,
} from '../../recoilModel';
import { CreateQnAModal } from '../../components/QnA';
import { triggerNotSupported } from '../../utils/dialogValidator';
Expand Down Expand Up @@ -133,7 +133,7 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
const schemas = useRecoilValue(schemasState(skillId ?? projectId));
const dialogs = useRecoilValue(validateDialogsSelectorFamily(skillId ?? projectId));
const skills = useRecoilValue(skillsStateSelector);
const displaySkillManifest = useRecoilValue(displaySkillManifestState(skillId ?? projectId));
const displaySkillManifestOnProjectId = useRecoilValue(displayManifestModalOnProjectIdSelector);
const skillsByProjectId = useRecoilValue(skillNameIdentifierByProjectIdSelector);
const projectDialogsMap = useRecoilValue(projectDialogsMapSelector);
const { startSingleBot, stopSingleBot } = useBotOperations();
Expand Down Expand Up @@ -728,7 +728,7 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
const handleDisplayManifestModal = (skillId: string) => {
const skillNameIdentifier = skillsByProjectId[skillId];
if (!skillNameIdentifier) return;
displayManifestModal(skillNameIdentifier, projectId);
displayManifestModal(skillNameIdentifier, skillId);
};

const selectedTrigger = currentDialog?.triggers.find((t) => t.id === selected);
Expand Down Expand Up @@ -882,11 +882,10 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
onSubmit={handleCreateQnA}
/>

{displaySkillManifest && (
{displaySkillManifestOnProjectId && (
<DisplayManifestModal
projectId={projectId}
skillNameIdentifier={displaySkillManifest}
onDismiss={() => dismissManifestModal(projectId)}
projectId={displaySkillManifestOnProjectId}
onDismiss={() => dismissManifestModal(displaySkillManifestOnProjectId)}
/>
)}
{brokenSkillInfo && (
Expand Down
25 changes: 23 additions & 2 deletions Composer/packages/client/src/recoilModel/selectors/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,42 @@ export const botProjectSpaceSelector = selector({
get: ({ get }) => {
const botProjects = get(botProjectIdsState);
const result = botProjects.map((projectId: string) => {
const { isRemote, isRootBot } = get(projectMetaDataState(projectId));
const dialogs = get(dialogsSelectorFamily(projectId));
const luFiles = get(luFilesState(projectId));
const lgFiles = get(lgFilesState(projectId));
const qnaFiles = get(qnaFilesState(projectId));
const formDialogSchemas = get(formDialogSchemasSelectorFamily(projectId));
const botProjectFile = get(botProjectFileState(projectId));
const metaData = get(projectMetaDataState(projectId));
const botError = get(botErrorState(projectId));
const buildEssentials = get(buildEssentialsSelector(projectId));
const name = get(botDisplayNameState(projectId));
const botNameId = get(botNameIdentifierState(projectId));
const setting = get(settingsState(projectId));
const skillManifests = get(skillManifestsState(projectId));
const dialogSchemas = get(dialogSchemasState(projectId));
const jsonSchemaFiles = get(jsonSchemaFilesState(projectId));
const schemas = get(schemasState(projectId));
const isPvaSchema = schemas && checkForPVASchema(schemas.sdk);

const diagnostics = BotIndexer.validate({ dialogs, setting, luFiles, lgFiles, qnaFiles, skillManifests });
const botAssets: BotAssets = {
projectId,
dialogs,
luFiles,
qnaFiles,
lgFiles,
skillManifests,
setting,
dialogSchemas,
formDialogSchemas,
botProjectFile,
jsonSchemaFiles,
recognizers: [],
crossTrainConfig: {},
};

const diagnostics = BotIndexer.validate({ ...botAssets, isRemote, isRootBot });

return {
dialogs,
Expand Down Expand Up @@ -139,6 +159,7 @@ export const botProjectDiagnosticsSelector = selector({
get: ({ get }) => {
const botProjects = get(botProjectIdsState);
const result = botProjects.map((projectId: string) => {
const { isRemote, isRootBot } = get(projectMetaDataState(projectId));
const dialogs = get(dialogsSelectorFamily(projectId));
const formDialogSchemas = get(formDialogSchemasSelectorFamily(projectId));
const luFiles = get(luFilesState(projectId));
Expand All @@ -164,7 +185,7 @@ export const botProjectDiagnosticsSelector = selector({
recognizers: [],
crossTrainConfig: {},
};
return BotIndexer.validate(botAssets);
return BotIndexer.validate({ ...botAssets, isRemote, isRootBot });
});
return result;
},
Expand Down
13 changes: 13 additions & 0 deletions Composer/packages/client/src/recoilModel/selectors/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
projectMetaDataState,
locationState,
botProjectIdsState,
displaySkillManifestState,
} from '../atoms';

export const skillsProjectIdSelector = selector({
Expand Down Expand Up @@ -79,3 +80,15 @@ export const skillNameIdentifierByProjectIdSelector = selector({
return skills;
},
});

// Display manifest modal if return is projectId
export const displayManifestModalOnProjectIdSelector = selector({
key: 'displayManifestModalOnProjectIdSelector',
get: ({ get }) => {
const botProjects = get(botProjectIdsState);
const displaySkillManifestOnProjects = botProjects.filter((projectId) => {
return get(displaySkillManifestState(projectId));
});
return displaySkillManifestOnProjects[0];
},
});
16 changes: 12 additions & 4 deletions Composer/packages/lib/indexers/__tests__/botIndexer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,20 @@ const botAssets: BotAssets = {
botProjectFile: {
id: 'test',
content: {
workspace: '',
name: '',
skills: {},
skills: {
'Email-Skill': {
workspace: '',
remote: false,
},
},
},
lastModified: '',
},
jsonSchemaFiles: [],
recognizers: [],
formDialogSchemas: [],
crossTrainConfig: {},
dialogSchemas: [],
qnaFiles: [],
lgFiles: [],
Expand All @@ -49,7 +57,7 @@ const botAssets: BotAssets = {
dialogs: [
{
luFile: 'a.lu',
skills: [`=settings.skill['Email-Skill'].endpointUrl`, `=settings.skill['Calendar-Skill'].endpointUrl`],
skills: ['Email-Skill', 'Calendar-Skill'],
} as DialogInfo,
],
setting: {
Expand Down Expand Up @@ -145,7 +153,7 @@ describe('checkSkillSetting', () => {
const errors = diagnostics.filter((item) => item.severity === DiagnosticSeverity.Error);
const warnings = diagnostics.filter((item) => item.severity === DiagnosticSeverity.Warning);
expect(errors.length).toEqual(1);
expect(warnings.length).toEqual(1);
expect(warnings.length).toEqual(0);
});
});

Expand Down
39 changes: 13 additions & 26 deletions Composer/packages/lib/indexers/src/botIndexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ import {
DiagnosticSeverity,
LuFile,
getSkillNameFromSetting,
fetchFromSettings,
SkillManifestFile,
DialogInfo,
DialogSetting,
LgFile,
QnAFile,
BotProjectFile,
} from '@bfc/shared';
import difference from 'lodash/difference';
import map from 'lodash/map';

import { getBaseName, getLocale } from './utils/help';

Expand Down Expand Up @@ -116,22 +115,16 @@ const checkLUISLocales = (assets: { dialogs: DialogInfo[]; setting: DialogSettin

/**
* Check bot skill & setting
* 1. used skill not existed in setting
* 2. appsettings.json Microsoft App Id or Skill Host Endpoint are empty
* 1. used skill not existed in *.botproj
*/
const checkSkillSetting = (assets: { dialogs: DialogInfo[]; setting: DialogSetting }): Diagnostic[] => {
const {
setting: { skill = {}, botId, skillHostEndpoint },
dialogs,
} = assets;
const checkSkillSetting = (assets: { dialogs: DialogInfo[]; botProjectFile: BotProjectFile }): Diagnostic[] => {
const { botProjectFile, dialogs } = assets;
const diagnostics: Diagnostic[] = [];

let skillUsed = false;
const skillNames = Object.keys(botProjectFile.content?.skills || {});
dialogs.forEach((dialog) => {
// used skill not existed in setting
dialog.skills.forEach((skillId) => {
const endpointUrlCollection = map(skill, ({ endpointUrl }) => endpointUrl);
if (!endpointUrlCollection.includes(fetchFromSettings(skillId, assets.setting))) {
if (!skillNames.includes(skillId)) {
const skillName = getSkillNameFromSetting(skillId) || skillId;
diagnostics.push(
new Diagnostic(
Expand All @@ -142,20 +135,8 @@ const checkSkillSetting = (assets: { dialogs: DialogInfo[]; setting: DialogSetti
);
}
});
if (dialog.skills.length) skillUsed = true;
});

// use skill require fill bot endpoint in skill page.
if (skillUsed && (!botId || !skillHostEndpoint)) {
diagnostics.push(
new Diagnostic(
'appsettings.json Microsoft App Id or Skill Host Endpoint are empty',
'appsettings.json',
DiagnosticSeverity.Warning
)
);
}

return diagnostics;
};

Expand All @@ -166,8 +147,14 @@ const validate = (assets: {
qnaFiles: QnAFile[];
setting: DialogSetting;
skillManifests: SkillManifestFile[];
botProjectFile: BotProjectFile;
isRemote?: boolean;
isRootBot?: boolean;
}): Diagnostic[] => {
return [...checkManifest(assets), ...checkSetting(assets), ...checkLUISLocales(assets), ...checkSkillSetting(assets)];
if (assets.isRemote) return [];
const settingDiagnostics = [...checkSetting(assets), ...checkLUISLocales(assets), ...checkSkillSetting(assets)];
if (assets.isRootBot) return settingDiagnostics;
return [...checkManifest(assets), ...settingDiagnostics];
};

const filterLUISFilesToPublish = (luFiles: LuFile[]): LuFile[] => {
Expand Down
Loading