From 56240da133777402057047223447816034878242 Mon Sep 17 00:00:00 2001 From: zeye Date: Tue, 23 Mar 2021 15:29:19 +0800 Subject: [PATCH 01/29] add placeholder for schema validator --- .../selectors/diagnosticsPageSelector.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts index b3ee13ac74..86265235ad 100644 --- a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts +++ b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts @@ -174,6 +174,18 @@ export const dialogsDiagnosticsSelectorFamily = selectorFamily({ }, }); +export const schemaDiagnosticsSelectorFamily = selectorFamily({ + key: 'schemaDiagnosticsSelectorFamily', + get: (projectId: string) => ({ get }) => { + const botAssets = get(botAssetsSelectFamily(projectId)); + if (botAssets === null) return []; + + // TODO: insert schema logic here: 1. policy 2. existence validation + console.log('try validate project schema diagnostics', projectId); + return []; + }, +}); + export const luDiagnosticsSelectorFamily = selectorFamily({ key: 'luDiagnosticsSelectorFamily', get: (projectId: string) => ({ get }) => { @@ -251,6 +263,7 @@ export const diagnosticsSelectorFamily = selectorFamily({ ...get(luDiagnosticsSelectorFamily(projectId)), ...get(lgDiagnosticsSelectorFamily(projectId)), ...get(qnaDiagnosticsSelectorFamily(projectId)), + ...get(schemaDiagnosticsSelectorFamily(projectId)), ], }); From bd7da86a8bc8e214913c78e44eb6619c14d61947 Mon Sep 17 00:00:00 2001 From: zeye Date: Thu, 25 Mar 2021 19:09:53 +0800 Subject: [PATCH 02/29] add schema validator pipeline with mocked fn --- .../selectors/diagnosticsPageSelector.ts | 21 ++++++++++++++----- .../lib/indexers/src/validations/index.ts | 2 ++ .../src/validations/schemaValidation/index.ts | 13 ++++++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts diff --git a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts index 86265235ad..bda75cb7a2 100644 --- a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts +++ b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { BotIndexer } from '@bfc/indexers'; +import { BotIndexer, validateSchema } from '@bfc/indexers'; import { selectorFamily, selector } from 'recoil'; import lodashGet from 'lodash/get'; import formatMessage from 'format-message'; @@ -170,7 +170,7 @@ export const dialogsDiagnosticsSelectorFamily = selectorFamily({ }); }); - return []; + return diagnosticList; }, }); @@ -178,11 +178,22 @@ export const schemaDiagnosticsSelectorFamily = selectorFamily({ key: 'schemaDiagnosticsSelectorFamily', get: (projectId: string) => ({ get }) => { const botAssets = get(botAssetsSelectFamily(projectId)); + // Why botAssets.dialogSchemas is a list? if (botAssets === null) return []; - // TODO: insert schema logic here: 1. policy 2. existence validation - console.log('try validate project schema diagnostics', projectId); - return []; + const rootProjectId = get(rootBotProjectIdSelector) ?? projectId; + + const sdkSchemaContent = botAssets.dialogSchemas[0]?.content; + if (!sdkSchemaContent) return []; + + const fullDiagnostics: DiagnosticInfo[] = []; + botAssets.dialogs.forEach((dialog) => { + const diagnostics = validateSchema(dialog.id, dialog.content, sdkSchemaContent); + fullDiagnostics.push( + ...diagnostics.map((d) => new DialogDiagnostic(rootProjectId, projectId, dialog.id, `${dialog.id}.dialog`, d)) + ); + }); + return fullDiagnostics; }, }); diff --git a/Composer/packages/lib/indexers/src/validations/index.ts b/Composer/packages/lib/indexers/src/validations/index.ts index d88ead354e..cc4b6a7919 100644 --- a/Composer/packages/lib/indexers/src/validations/index.ts +++ b/Composer/packages/lib/indexers/src/validations/index.ts @@ -69,3 +69,5 @@ export function validateDialog( return { diagnostics: [new Diagnostic(error.message, id)], cache }; } } + +export { validateSchema } from './schemaValidation'; diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts new file mode 100644 index 0000000000..1109ca38d2 --- /dev/null +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +import { MicrosoftIDialog, Diagnostic } from '@bfc/shared'; +import { SchemaDefinitions } from '@bfc/shared/lib/schemaUtils/types'; + +export const validateSchema = ( + dialogId: string, + dialogData: MicrosoftIDialog, + schema: SchemaDefinitions +): Diagnostic[] => { + console.log('id, data, schema', dialogId, dialogData, schema); + return []; +}; From f854c39e97f9feb3cae84ddfc8e9db9275e8a881 Mon Sep 17 00:00:00 2001 From: zeye Date: Thu, 1 Apr 2021 20:33:01 +0800 Subject: [PATCH 03/29] add schema visitor --- .../schemaValidation/__mocks__/dialogMocks.ts | 57 +++++ .../schemaValidation/__mocks__/sdkSchema.ts | 20 ++ .../__mocks__/sdkSchemaMocks.ts | 212 ++++++++++++++++++ .../schemaValidation/schemaUtils.test.ts | 26 +++ .../walkAdaptiveDialog.test.ts | 28 +++ .../src/validations/schemaValidation/index.ts | 7 +- .../schemaValidation/schemaUtils.ts | 56 +++++ .../schemaValidation/walkAdaptiveDialog.ts | 124 ++++++++++ Composer/packages/types/src/schema.ts | 2 +- Composer/packages/types/src/sdk.ts | 6 +- 10 files changed, 532 insertions(+), 6 deletions(-) create mode 100644 Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts create mode 100644 Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchema.ts create mode 100644 Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts create mode 100644 Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts create mode 100644 Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts create mode 100644 Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts create mode 100644 Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts new file mode 100644 index 0000000000..5c1b5c0710 --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +export const simpleGreeting: any = { + $kind: 'Microsoft.AdaptiveDialog', + $designer: { + $designer: { + name: 'EmptyBot-1', + description: '', + id: '47yxe0', + }, + }, + autoEndDialog: true, + defaultResultProperty: 'dialog.result', + triggers: [ + { + $kind: 'Microsoft.OnConversationUpdateActivity', + $designer: { + id: '376720', + }, + actions: [ + { + $kind: 'Microsoft.SwitchCondition', + $designer: { + id: 'sJzdQm', + }, + cases: [ + { + value: 'asd', + actions: [ + { + $kind: 'Microsoft.SendActivity', + $designer: { + id: 'AwT1u7', + }, + }, + ], + }, + ], + default: [ + { + $kind: 'Microsoft.SendActivity', + $designer: { + id: 'rMLkPc', + }, + }, + ], + }, + ], + }, + ], + $schema: + 'https://raw.githubusercontent.com/microsoft/BotFramework-Composer/stable/Composer/packages/server/schemas/sdk.schema', + generator: 'EmptyBot-1.lg', + id: 'EmptyBot-1', + recognizer: 'EmptyBot-1.lu.qna', +}; diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchema.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchema.ts new file mode 100644 index 0000000000..de22165971 --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchema.ts @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { SDKKinds } from '@botframework-composer/types'; + +import { + AdaptiveDialogSchema, + IfConditionSchema, + OnConvUpdateSchema, + OnDialogEventSchema, + SwitchConditionSchema, +} from './sdkSchemaMocks'; + +export const sdkSchemaDefinitionMock = { + [SDKKinds.SwitchCondition]: SwitchConditionSchema, + [SDKKinds.IfCondition]: IfConditionSchema, + [SDKKinds.AdaptiveDialog]: AdaptiveDialogSchema, + [SDKKinds.OnDialogEvent]: OnDialogEventSchema, + [SDKKinds.OnConversationUpdateActivity]: OnConvUpdateSchema, +}; diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts new file mode 100644 index 0000000000..ee8b7dd371 --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts @@ -0,0 +1,212 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { JSONSchema7 } from '@botframework-composer/types'; + +export const AdaptiveDialogSchema: JSONSchema7 = { + $schema: 'https://schemas.botframework.com/schemas/component/v1.0/component.schema', + $role: 'implements(Microsoft.IDialog)', + title: 'Adaptive dialog', + description: 'Flexible, data driven dialog that can adapt to the conversation.', + type: 'object', + properties: { + id: { + type: 'string', + pattern: '^(?!(=)).*', + title: 'Id', + description: 'Optional dialog ID.', + }, + autoEndDialog: { + $ref: 'schema:#/definitions/booleanExpression', + title: 'Auto end dialog', + description: + 'If set to true the dialog will automatically end when there are no further actions. If set to false, remember to manually end the dialog using EndDialog action.', + default: true, + }, + defaultResultProperty: { + type: 'string', + title: 'Default result property', + description: 'Value that will be passed back to the parent dialog.', + default: 'dialog.result', + }, + dialogs: { + type: 'array', + title: 'Dialogs added to DialogSet', + items: { + $kind: 'Microsoft.IDialog', + title: 'Dialog', + description: 'Dialogs will be added to DialogSet.', + }, + }, + recognizer: { + $kind: 'Microsoft.IRecognizer', + title: 'Recognizer', + description: 'Input recognizer that interprets user input into intent and entities.', + }, + generator: { + $kind: 'Microsoft.ILanguageGenerator', + title: 'Language generator', + description: 'Language generator that generates bot responses.', + }, + selector: { + $kind: 'Microsoft.ITriggerSelector', + title: 'Selector', + description: "Policy to determine which trigger is executed. Defaults to a 'best match' selector (optional).", + }, + triggers: { + type: 'array', + description: 'List of triggers defined for this dialog.', + title: 'Triggers', + items: { + $kind: 'Microsoft.ITrigger', + title: 'Event triggers', + description: 'Event triggers for handling events.', + }, + }, + schema: { + title: 'Schema', + description: 'Schema to fill in.', + anyOf: [ + { + $ref: 'http://json-schema.org/draft-07/schema#', + }, + { + type: 'string', + title: 'Reference to JSON schema', + description: 'Reference to JSON schema .dialog file.', + }, + ], + }, + }, +}; + +export const IfConditionSchema: JSONSchema7 = { + $schema: 'https://schemas.botframework.com/schemas/component/v1.0/component.schema', + $role: 'implements(Microsoft.IDialog)', + title: 'If condition', + description: 'Two-way branch the conversation flow based on a condition.', + type: 'object', + required: ['condition'], + properties: { + id: { + type: 'string', + title: 'Id', + description: 'Optional id for the dialog', + }, + condition: { + $ref: 'schema:#/definitions/condition', + title: 'Condition', + description: 'Expression to evaluate.', + examples: ['user.age > 3'], + }, + disabled: { + $ref: 'schema:#/definitions/booleanExpression', + title: 'Disabled', + description: 'Optional condition which if true will disable this action.', + examples: [true, '=user.age > 3'], + }, + actions: { + type: 'array', + items: { + $kind: 'Microsoft.IDialog', + }, + title: 'Actions', + description: 'Actions to execute if condition is true.', + }, + elseActions: { + type: 'array', + items: { + $kind: 'Microsoft.IDialog', + }, + title: 'Else', + description: 'Actions to execute if condition is false.', + }, + }, +}; + +export const OnConvUpdateSchema: JSONSchema7 = { + $schema: 'https://schemas.botframework.com/schemas/component/v1.0/component.schema', + $role: ['implements(Microsoft.ITrigger)', 'extends(Microsoft.OnCondition)'], + title: 'On ConversationUpdate activity', + description: "Actions to perform on receipt of an activity with type 'ConversationUpdate'.", + type: 'object', + required: [], +}; +export const OnDialogEventSchema: JSONSchema7 = { + $schema: 'https://schemas.botframework.com/schemas/component/v1.0/component.schema', + $role: ['implements(Microsoft.ITrigger)', 'extends(Microsoft.OnCondition)'], + title: 'On dialog event', + description: 'Actions to perform when a specific dialog event occurs.', + type: 'object', + properties: { + event: { + type: 'string', + title: 'Dialog event name', + description: 'Name of dialog event.', + }, + }, + required: ['event'], +}; + +export const SwitchConditionSchema: JSONSchema7 = { + $schema: 'https://schemas.botframework.com/schemas/component/v1.0/component.schema', + $role: 'implements(Microsoft.IDialog)', + title: 'Switch condition', + description: 'Execute different actions based on the value of a property.', + type: 'object', + required: ['condition'], + properties: { + id: { + type: 'string', + title: 'Id', + description: 'Optional id for the dialog', + }, + condition: { + $ref: 'schema:#/definitions/stringExpression', + title: 'Condition', + description: 'Property to evaluate.', + examples: ['user.favColor'], + }, + disabled: { + $ref: 'schema:#/definitions/booleanExpression', + title: 'Disabled', + description: 'Optional condition which if true will disable this action.', + examples: [true, '=user.age > 3'], + }, + cases: { + type: 'array', + title: 'Cases', + description: 'Actions for each possible condition.', + items: { + type: 'object', + title: 'Case', + description: 'Case and actions.', + properties: { + value: { + type: ['number', 'integer', 'boolean', 'string'], + title: 'Value', + description: 'The value to compare the condition with.', + examples: ['red', 'true', '13'], + }, + actions: { + type: 'array', + items: { + $kind: 'Microsoft.IDialog', + }, + title: 'Actions', + description: 'Actions to execute.', + }, + }, + required: ['value'], + }, + }, + default: { + type: 'array', + items: { + $kind: 'Microsoft.IDialog', + }, + title: 'Default', + description: 'Actions to execute if none of the cases meet the condition.', + }, + }, +}; diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts new file mode 100644 index 0000000000..7be87f5c34 --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { discoverNestedProperties, isTrigger } from '../../../src/validations/schemaValidation/schemaUtils'; + +import { + AdaptiveDialogSchema, + IfConditionSchema, + OnDialogEventSchema, + SwitchConditionSchema, +} from './__mocks__/sdkSchemaMocks'; + +describe('#schemaUtils', () => { + it('isTrigger() should recognizer trigger schema.', () => { + expect(isTrigger(OnDialogEventSchema)).toBeTruthy(); + expect(isTrigger(IfConditionSchema)).toBeFalsy(); + expect(isTrigger(AdaptiveDialogSchema)).toBeFalsy(); + }); + + it('discoverNestedProperties() should find correct property names.', () => { + expect(discoverNestedProperties(AdaptiveDialogSchema)).toEqual(expect.arrayContaining(['triggers', 'dialogs'])); + expect(discoverNestedProperties(OnDialogEventSchema)).toEqual(expect.arrayContaining(['actions'])); + expect(discoverNestedProperties(IfConditionSchema)).toEqual(expect.arrayContaining(['actions', 'elseActions'])); + expect(discoverNestedProperties(SwitchConditionSchema)).toEqual(expect.arrayContaining(['cases', 'default'])); + }); +}); diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts new file mode 100644 index 0000000000..1cb99514fd --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { SDKKinds } from '@botframework-composer/types'; + +import { walkAdaptiveDialog } from '../../../src/validations/schemaValidation/walkAdaptiveDialog'; + +import { simpleGreeting } from './__mocks__/dialogMocks'; +import { sdkSchemaDefinitionMock } from './__mocks__/sdkSchema'; + +describe('visitAdaptiveDialog', () => { + it('should visit every adaptive elements in `simpleGreeting`', () => { + const result: any = {}; + walkAdaptiveDialog(simpleGreeting, sdkSchemaDefinitionMock, ($kind, _, path) => { + result[path] = $kind; + return true; + }); + expect(result).toEqual( + expect.objectContaining({ + '': SDKKinds.AdaptiveDialog, + 'triggers[0]': SDKKinds.OnConversationUpdateActivity, + 'triggers[0].actions[0]': SDKKinds.SwitchCondition, + 'triggers[0].actions[0].default[0]': SDKKinds.SendActivity, + 'triggers[0].actions[0].cases[0].actions[0]': SDKKinds.SendActivity, + }) + ); + }); +}); diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts index 1109ca38d2..8975eefac2 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts @@ -1,13 +1,16 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. import { MicrosoftIDialog, Diagnostic } from '@bfc/shared'; -import { SchemaDefinitions } from '@bfc/shared/lib/schemaUtils/types'; +import { SchemaDefinitions } from '@botframework-composer/types'; + +import { walkAdaptiveDialog } from './walkAdaptiveDialog'; export const validateSchema = ( dialogId: string, dialogData: MicrosoftIDialog, schema: SchemaDefinitions ): Diagnostic[] => { - console.log('id, data, schema', dialogId, dialogData, schema); + walkAdaptiveDialog; + console.log('id, data, schema', dialogId, dialogData, schema.definitions); return []; }; diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts new file mode 100644 index 0000000000..0489848c98 --- /dev/null +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { JSONSchema7 } from '@botframework-composer/types'; + +export const isTrigger = (schema: JSONSchema7): boolean => { + const roles = typeof schema.$role === 'string' ? [schema.$role] : schema.$role ?? []; + + return roles.some((roleString) => { + return roleString.indexOf('implements(Microsoft.ITrigger)') > -1; + }); +}; + +const triggerNesterProperties = ['actions']; +export const discoverNestedProperties = (schema: JSONSchema7): string[] => { + if (isTrigger(schema)) return triggerNesterProperties; + if (!schema.properties) return []; + + return Object.entries(schema.properties) + .filter(([, propertyDef]) => { + const { type, items } = propertyDef as any; + /** + * Discover child elements (triggers, actions). For example: + * 1. In Microsoft.IfCondition.schema + * ```json + * properties.actions = { + * "type": "array", + * "items": { + * "$kind": "Microsoft.IDialog" + * }, + * "title": "Actions", + * "description": "Actions to execute if condition is true." + * } + * ``` + * Returns ["actions"]. + * + * 2. In Microsoft.AdaptiveDialog.schema + * ```json + * properties.triggers = { + * "type": "array", + * "description": "List of triggers defined for this dialog.", + * "title": "Triggers", + * "items": { + * "$kind": "Microsoft.ITrigger", + * "title": "Event triggers", + * "description": "Event triggers for handling events." + * } + * } + * ``` + * Returns ["triggers"]. + */ + const hasNestedAdaptiveElements = type === 'array' && (Boolean(items?.$kind) || Boolean(items?.properties)); + return hasNestedAdaptiveElements; + }) + .map(([propertyName]) => propertyName); +}; diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts new file mode 100644 index 0000000000..a4d7bda090 --- /dev/null +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts @@ -0,0 +1,124 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + MicrosoftAdaptiveDialog, + BaseSchema, + SchemaDefinitions, + SDKKinds, + SwitchCondition, +} from '@botframework-composer/types'; + +import { discoverNestedProperties } from './schemaUtils'; + +// returns true to continue the visit. +type VisitAdaptiveComponentFn = ($kind: SDKKinds | string, data: BaseSchema, path: string) => boolean; + +export const walkAdaptiveDialog = ( + adaptiveDialog: MicrosoftAdaptiveDialog, + sdkSchema: SchemaDefinitions, + fn: VisitAdaptiveComponentFn +): boolean => { + return walkWithPath(adaptiveDialog, sdkSchema, '', '', fn); +}; + +const joinPath = (parentPath: string, currentKey: string | number): string => { + if (typeof currentKey === 'string') { + return parentPath ? `${parentPath}.${currentKey}` : currentKey; + } + + if (typeof currentKey === 'number') { + return parentPath ? `${parentPath}[${currentKey}]` : `[${currentKey}]`; + } + + return ''; +}; + +const walkWithPath = ( + adaptiveData: BaseSchema, + sdkSchema: SchemaDefinitions, + parentPath: string, + currentKey: string | number, + fn: VisitAdaptiveComponentFn +): boolean => { + const { $kind } = adaptiveData; + + // SwitchCondition needs to be handled specially due to 'CaseCondition' doesn't conains a $kind property but is iterable. + // Reference: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Actions/Case.cs + if ($kind === SDKKinds.SwitchCondition) { + return walkSwitchCondition(adaptiveData, sdkSchema, parentPath, currentKey, fn); + } + + // Visit current data before schema validation to make sure all $kind blocks are visited. + const currentPath = joinPath(parentPath, currentKey); + fn($kind, adaptiveData, currentPath); + + const schema = sdkSchema[$kind]; + const nestedProperties = schema ? discoverNestedProperties(schema) : adaptiveData.actions ? ['actions'] : []; + if (nestedProperties.length === 0) return true; + + /** + * Examples of nested properties in built-in $kinds: + * 1. ['actions'] in every Trigger $kind + * 2. ['actions'] in Foreach, ForeachPage + * 3. ['actions', 'elseActions'] in IfCondition + * 4. ['cases', 'default'] in SwitchCondition + */ + for (const propName of nestedProperties) { + const childElements = adaptiveData[propName]; + if (!Array.isArray(childElements)) { + continue; + } + + /** + * Visit nested adaptive elements. For example: + * 1. Triggers under Dialog; + * 2. Actions under Trigger; + * 3. Actions under Action. + */ + for (let i = 0; i < childElements.length; i++) { + const arrayPath = joinPath(currentPath, propName); + const shouldContinue = walkWithPath(childElements[i], sdkSchema, arrayPath, i, fn); + if (!shouldContinue) return false; + } + } + + return true; +}; + +const walkSwitchCondition = ( + switchConditionData: SwitchCondition, + sdkSchema: SchemaDefinitions, + parrentPath: string, + currentKey: string | number, + fn: VisitAdaptiveComponentFn +): boolean => { + const currentPath = joinPath(parrentPath, currentKey); + fn(SDKKinds.SwitchCondition, switchConditionData, currentPath); + + const defaultActions = switchConditionData.default; + + // Visit the 'default' properties + if (Array.isArray(defaultActions)) { + for (let i = 0; i < defaultActions.length; i++) { + const shouldContinue = walkWithPath(defaultActions[i], sdkSchema, joinPath(currentPath, 'default'), i, fn); + if (!shouldContinue) return false; + } + } + + const casesData = switchConditionData.cases; + if (Array.isArray(casesData)) { + // Expand cases data + for (let iCase = 0; iCase < casesData.length; iCase++) { + const caseActions = casesData[iCase].actions; + if (!Array.isArray(caseActions)) continue; + + // Expand actions under a case. + for (let i = 0; i < caseActions.length; i++) { + const shouldContinue = walkWithPath(caseActions[i], sdkSchema, `${currentPath}.cases[${iCase}].actions`, i, fn); + if (!shouldContinue) return false; + } + } + } + return true; +}; diff --git a/Composer/packages/types/src/schema.ts b/Composer/packages/types/src/schema.ts index 08699b4466..eb9ffc7b0d 100644 --- a/Composer/packages/types/src/schema.ts +++ b/Composer/packages/types/src/schema.ts @@ -149,7 +149,7 @@ interface AdaptiveSchema extends Omit Date: Thu, 1 Apr 2021 21:15:24 +0800 Subject: [PATCH 04/29] display diagnostics data in debug panel --- .../src/validations/schemaValidation/index.ts | 25 ++++++++++++++----- .../schemaValidation/walkAdaptiveDialog.ts | 1 + 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts index 8975eefac2..9338406acf 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts @@ -1,16 +1,29 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { MicrosoftIDialog, Diagnostic } from '@bfc/shared'; -import { SchemaDefinitions } from '@botframework-composer/types'; +import { Diagnostic } from '@bfc/shared'; +import formatMessage from 'format-message'; +import { DiagnosticSeverity, MicrosoftAdaptiveDialog, SchemaDefinitions } from '@botframework-composer/types'; import { walkAdaptiveDialog } from './walkAdaptiveDialog'; +const SCHEMA_NOT_FOUND = formatMessage('Schema definition not found in sdk.schema.'); + export const validateSchema = ( dialogId: string, - dialogData: MicrosoftIDialog, + dialogData: MicrosoftAdaptiveDialog, schema: SchemaDefinitions ): Diagnostic[] => { - walkAdaptiveDialog; - console.log('id, data, schema', dialogId, dialogData, schema.definitions); - return []; + const diagnostics: Diagnostic[] = []; + const schemas: any = schema.definitions ?? {}; + + walkAdaptiveDialog(dialogData, schemas, ($kind, data, path) => { + if (!schemas[$kind]) { + diagnostics.push( + new Diagnostic(`${$kind}: ${SCHEMA_NOT_FOUND}`, `${dialogId}.dialog`, DiagnosticSeverity.Error, path) + ); + } + return true; + }); + + return diagnostics; }; diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts index a4d7bda090..c35344e3c4 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts @@ -86,6 +86,7 @@ const walkWithPath = ( return true; }; +// TODO: optimize the discoverNestedProperties handler to makes cases[i].actions discoverable. const walkSwitchCondition = ( switchConditionData: SwitchCondition, sdkSchema: SchemaDefinitions, From c6984b8584f2b61f7cb6bd11fc5a20e59c97b5db Mon Sep 17 00:00:00 2001 From: zeye Date: Thu, 1 Apr 2021 21:28:10 +0800 Subject: [PATCH 05/29] revert sdk.ts --- Composer/packages/types/src/sdk.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Composer/packages/types/src/sdk.ts b/Composer/packages/types/src/sdk.ts index 0a56f67464..9a305c4b16 100644 --- a/Composer/packages/types/src/sdk.ts +++ b/Composer/packages/types/src/sdk.ts @@ -213,7 +213,7 @@ export enum DialogEvent { stepsResumed = 'stepsResumed', } -export type MicrosoftITrigger = BaseSchema & { +type RuleBase = BaseSchema & { /** Optional constraint to which must be met for this rule to fire */ constraint?: string; /** Sequence of steps or dialogs to execute */ @@ -221,7 +221,7 @@ export type MicrosoftITrigger = BaseSchema & { }; /** This defines the steps to take when an Intent is recognized (and optionally entities) */ -export type OnIntent = MicrosoftITrigger & { +export type OnIntent = RuleBase & { /** Intent name to trigger on */ intent: string; /** The entities required to trigger this rule */ @@ -229,7 +229,7 @@ export type OnIntent = MicrosoftITrigger & { }; /** Defines a sequence of steps to take if there is no other trigger or plan operating */ -export type OnUnknownIntent = MicrosoftITrigger & {}; +export type OnUnknownIntent = RuleBase & {}; export type ITriggerCondition = OnIntent | OnUnknownIntent; From c84f5179a17a5585ae3f2dbbc00e7d0422ff303b Mon Sep 17 00:00:00 2001 From: zeye Date: Thu, 1 Apr 2021 21:29:51 +0800 Subject: [PATCH 06/29] decrease schema diagnostic severity to 'Warning' --- .../lib/indexers/src/validations/schemaValidation/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts index 9338406acf..f6b7bd2889 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts @@ -19,7 +19,7 @@ export const validateSchema = ( walkAdaptiveDialog(dialogData, schemas, ($kind, data, path) => { if (!schemas[$kind]) { diagnostics.push( - new Diagnostic(`${$kind}: ${SCHEMA_NOT_FOUND}`, `${dialogId}.dialog`, DiagnosticSeverity.Error, path) + new Diagnostic(`${$kind}: ${SCHEMA_NOT_FOUND}`, `${dialogId}.dialog`, DiagnosticSeverity.Warning, path) ); } return true; From 7b27afd1a1245ec1d2b28505c156de2875fea0fd Mon Sep 17 00:00:00 2001 From: zeye Date: Fri, 2 Apr 2021 16:24:11 +0800 Subject: [PATCH 07/29] optmize path join logic --- .../schemaValidation/walkAdaptiveDialog.ts | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts index c35344e3c4..e2da267538 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts @@ -12,7 +12,12 @@ import { import { discoverNestedProperties } from './schemaUtils'; // returns true to continue the visit. -type VisitAdaptiveComponentFn = ($kind: SDKKinds | string, data: BaseSchema, path: string) => boolean; +type VisitAdaptiveComponentFn = ( + $kind: SDKKinds | string, + data: BaseSchema, + currentPath: string, + parentPath: string +) => boolean; export const walkAdaptiveDialog = ( adaptiveDialog: MicrosoftAdaptiveDialog, @@ -37,8 +42,8 @@ const joinPath = (parentPath: string, currentKey: string | number): string => { const walkWithPath = ( adaptiveData: BaseSchema, sdkSchema: SchemaDefinitions, + currentPath: string, parentPath: string, - currentKey: string | number, fn: VisitAdaptiveComponentFn ): boolean => { const { $kind } = adaptiveData; @@ -46,12 +51,11 @@ const walkWithPath = ( // SwitchCondition needs to be handled specially due to 'CaseCondition' doesn't conains a $kind property but is iterable. // Reference: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Actions/Case.cs if ($kind === SDKKinds.SwitchCondition) { - return walkSwitchCondition(adaptiveData, sdkSchema, parentPath, currentKey, fn); + return walkSwitchCondition(adaptiveData, sdkSchema, currentPath, parentPath, fn); } // Visit current data before schema validation to make sure all $kind blocks are visited. - const currentPath = joinPath(parentPath, currentKey); - fn($kind, adaptiveData, currentPath); + fn($kind, adaptiveData, currentPath, parentPath); const schema = sdkSchema[$kind]; const nestedProperties = schema ? discoverNestedProperties(schema) : adaptiveData.actions ? ['actions'] : []; @@ -77,8 +81,13 @@ const walkWithPath = ( * 3. Actions under Action. */ for (let i = 0; i < childElements.length; i++) { - const arrayPath = joinPath(currentPath, propName); - const shouldContinue = walkWithPath(childElements[i], sdkSchema, arrayPath, i, fn); + const shouldContinue = walkWithPath( + childElements[i], + sdkSchema, + joinPath(currentPath, `${propName}[${i}]`), + currentPath, + fn + ); if (!shouldContinue) return false; } } @@ -90,19 +99,24 @@ const walkWithPath = ( const walkSwitchCondition = ( switchConditionData: SwitchCondition, sdkSchema: SchemaDefinitions, + currentPath: string, parrentPath: string, - currentKey: string | number, fn: VisitAdaptiveComponentFn ): boolean => { - const currentPath = joinPath(parrentPath, currentKey); - fn(SDKKinds.SwitchCondition, switchConditionData, currentPath); + fn(SDKKinds.SwitchCondition, switchConditionData, currentPath, parrentPath); const defaultActions = switchConditionData.default; // Visit the 'default' properties if (Array.isArray(defaultActions)) { for (let i = 0; i < defaultActions.length; i++) { - const shouldContinue = walkWithPath(defaultActions[i], sdkSchema, joinPath(currentPath, 'default'), i, fn); + const shouldContinue = walkWithPath( + defaultActions[i], + sdkSchema, + joinPath(currentPath, `default[${i}]`), + currentPath, + fn + ); if (!shouldContinue) return false; } } @@ -116,7 +130,13 @@ const walkSwitchCondition = ( // Expand actions under a case. for (let i = 0; i < caseActions.length; i++) { - const shouldContinue = walkWithPath(caseActions[i], sdkSchema, `${currentPath}.cases[${iCase}].actions`, i, fn); + const shouldContinue = walkWithPath( + caseActions[i], + sdkSchema, + `${currentPath}.cases[${iCase}].actions[${i}]`, + currentPath, + fn + ); if (!shouldContinue) return false; } } From 733f68f1cbc79c53f25cc3ca8835f681d2059ad5 Mon Sep 17 00:00:00 2001 From: zeye Date: Fri, 2 Apr 2021 17:30:20 +0800 Subject: [PATCH 08/29] impl a unified walker covers SwitchCondition --- .../schemaValidation/__mocks__/dialogMocks.ts | 69 +++++------ .../schemaValidation/schemaUtils.test.ts | 17 ++- .../walkAdaptiveDialog.test.ts | 4 +- .../schemaValidation/schemaUtils.ts | 115 ++++++++++++------ .../schemaValidation/walkAdaptiveDialog.ts | 77 ++---------- 5 files changed, 129 insertions(+), 153 deletions(-) diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts index 5c1b5c0710..d3c7c1a8df 100644 --- a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/dialogMocks.ts @@ -1,7 +1,36 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -export const simpleGreeting: any = { +export const sendActivityStub = { + $kind: 'Microsoft.SendActivity', + $designer: { + id: 'AwT1u7', + }, +}; + +export const switchConditionStub = { + $kind: 'Microsoft.SwitchCondition', + $designer: { + id: 'sJzdQm', + }, + default: [sendActivityStub], + cases: [ + { + value: 'case1', + actions: [sendActivityStub], + }, + ], +}; + +export const onConversationUpdateActivityStub = { + $kind: 'Microsoft.OnConversationUpdateActivity', + $designer: { + id: '376720', + }, + actions: [switchConditionStub], +}; + +export const simpleGreetingDialog: any = { $kind: 'Microsoft.AdaptiveDialog', $designer: { $designer: { @@ -12,43 +41,7 @@ export const simpleGreeting: any = { }, autoEndDialog: true, defaultResultProperty: 'dialog.result', - triggers: [ - { - $kind: 'Microsoft.OnConversationUpdateActivity', - $designer: { - id: '376720', - }, - actions: [ - { - $kind: 'Microsoft.SwitchCondition', - $designer: { - id: 'sJzdQm', - }, - cases: [ - { - value: 'asd', - actions: [ - { - $kind: 'Microsoft.SendActivity', - $designer: { - id: 'AwT1u7', - }, - }, - ], - }, - ], - default: [ - { - $kind: 'Microsoft.SendActivity', - $designer: { - id: 'rMLkPc', - }, - }, - ], - }, - ], - }, - ], + triggers: [onConversationUpdateActivityStub], $schema: 'https://raw.githubusercontent.com/microsoft/BotFramework-Composer/stable/Composer/packages/server/schemas/sdk.schema', generator: 'EmptyBot-1.lg', diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts index 7be87f5c34..6c9c501388 100644 --- a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts @@ -1,11 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { discoverNestedProperties, isTrigger } from '../../../src/validations/schemaValidation/schemaUtils'; +import { discoverNestedPaths, isTrigger } from '../../../src/validations/schemaValidation/schemaUtils'; +import { onConversationUpdateActivityStub, simpleGreetingDialog, switchConditionStub } from './__mocks__/dialogMocks'; import { AdaptiveDialogSchema, IfConditionSchema, + OnConvUpdateSchema, OnDialogEventSchema, SwitchConditionSchema, } from './__mocks__/sdkSchemaMocks'; @@ -18,9 +20,14 @@ describe('#schemaUtils', () => { }); it('discoverNestedProperties() should find correct property names.', () => { - expect(discoverNestedProperties(AdaptiveDialogSchema)).toEqual(expect.arrayContaining(['triggers', 'dialogs'])); - expect(discoverNestedProperties(OnDialogEventSchema)).toEqual(expect.arrayContaining(['actions'])); - expect(discoverNestedProperties(IfConditionSchema)).toEqual(expect.arrayContaining(['actions', 'elseActions'])); - expect(discoverNestedProperties(SwitchConditionSchema)).toEqual(expect.arrayContaining(['cases', 'default'])); + expect(discoverNestedPaths(simpleGreetingDialog, AdaptiveDialogSchema)).toEqual( + expect.arrayContaining(['triggers']) + ); + expect(discoverNestedPaths(onConversationUpdateActivityStub, OnConvUpdateSchema)).toEqual( + expect.arrayContaining(['actions']) + ); + expect(discoverNestedPaths(switchConditionStub, SwitchConditionSchema)).toEqual( + expect.arrayContaining(['cases[0].actions', 'default']) + ); }); }); diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts index 1cb99514fd..9ecb75e4f9 100644 --- a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/walkAdaptiveDialog.test.ts @@ -5,13 +5,13 @@ import { SDKKinds } from '@botframework-composer/types'; import { walkAdaptiveDialog } from '../../../src/validations/schemaValidation/walkAdaptiveDialog'; -import { simpleGreeting } from './__mocks__/dialogMocks'; +import { simpleGreetingDialog } from './__mocks__/dialogMocks'; import { sdkSchemaDefinitionMock } from './__mocks__/sdkSchema'; describe('visitAdaptiveDialog', () => { it('should visit every adaptive elements in `simpleGreeting`', () => { const result: any = {}; - walkAdaptiveDialog(simpleGreeting, sdkSchemaDefinitionMock, ($kind, _, path) => { + walkAdaptiveDialog(simpleGreetingDialog, sdkSchemaDefinitionMock, ($kind, _, path) => { result[path] = $kind; return true; }); diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts index 0489848c98..3c1ae86b22 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts @@ -1,7 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { JSONSchema7 } from '@botframework-composer/types'; +import { BaseSchema, JSONSchema7 } from '@botframework-composer/types'; +import get from 'lodash/get'; export const isTrigger = (schema: JSONSchema7): boolean => { const roles = typeof schema.$role === 'string' ? [schema.$role] : schema.$role ?? []; @@ -12,45 +13,81 @@ export const isTrigger = (schema: JSONSchema7): boolean => { }; const triggerNesterProperties = ['actions']; -export const discoverNestedProperties = (schema: JSONSchema7): string[] => { + +const propertyDefinesActionArray = (propertyDefinition: JSONSchema7): boolean => { + const { type, items } = propertyDefinition; + return type === 'array' && Boolean(get(items, '$kind')); +}; + +export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): string[] => { if (isTrigger(schema)) return triggerNesterProperties; if (!schema.properties) return []; - return Object.entries(schema.properties) - .filter(([, propertyDef]) => { - const { type, items } = propertyDef as any; - /** - * Discover child elements (triggers, actions). For example: - * 1. In Microsoft.IfCondition.schema - * ```json - * properties.actions = { - * "type": "array", - * "items": { - * "$kind": "Microsoft.IDialog" - * }, - * "title": "Actions", - * "description": "Actions to execute if condition is true." - * } - * ``` - * Returns ["actions"]. - * - * 2. In Microsoft.AdaptiveDialog.schema - * ```json - * properties.triggers = { - * "type": "array", - * "description": "List of triggers defined for this dialog.", - * "title": "Triggers", - * "items": { - * "$kind": "Microsoft.ITrigger", - * "title": "Event triggers", - * "description": "Event triggers for handling events." - * } - * } - * ``` - * Returns ["triggers"]. - */ - const hasNestedAdaptiveElements = type === 'array' && (Boolean(items?.$kind) || Boolean(items?.properties)); - return hasNestedAdaptiveElements; - }) - .map(([propertyName]) => propertyName); + const nestedPaths: string[] = []; + + const entries = Object.entries(schema.properties); + for (const entry of entries) { + const [propertyName, propertyDef] = entry; + const { type, items } = propertyDef as any; + + /** + * Discover child elements (triggers, actions). For example: + * 1. In Microsoft.IfCondition.schema + * ```json + * properties.actions = { + * "type": "array", + * "items": { + * "$kind": "Microsoft.IDialog" + * }, + * "title": "Actions", + * "description": "Actions to execute if condition is true." + * } + * ``` + * Returns ["actions"]. + * + * 2. In Microsoft.AdaptiveDialog.schema + * ```json + * properties.triggers = { + * "type": "array", + * "description": "List of triggers defined for this dialog.", + * "title": "Triggers", + * "items": { + * "$kind": "Microsoft.ITrigger", + * "title": "Event triggers", + * "description": "Event triggers for handling events." + * } + * } + * ``` + * Returns ["triggers"]. + */ + const propertyData = get(data, propertyName); + if (!Array.isArray(propertyData) || !propertyData.length) continue; + + const isSchemaNested = propertyDefinesActionArray(propertyDef); + const dataContainsAction = Boolean(propertyData[0].$kind); + + if (isSchemaNested && dataContainsAction) { + nestedPaths.push(propertyName); + continue; + } + + const schemaHasSkipLevelActions = type === 'array' && propertyDefinesActionArray(get(items, 'properties.actions')); + + /** + * Discover skip-level child elements. Currently, this logic is for handling SwitchCondition. + * Reference to SwitchCondition.schema: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Actions/Microsoft.SwitchCondition.schema + */ + if (schemaHasSkipLevelActions) { + propertyData.forEach((caseData, caseIndex) => { + const caseActions = caseData.actions; + if (!Array.isArray(caseActions) || !caseActions.length) return; + + for (let i = 0; i < caseActions.length; i++) { + nestedPaths.push(`${propertyName}[${caseIndex}].actions`); + } + }); + } + } + + return nestedPaths; }; diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts index e2da267538..e2ee4625c1 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts @@ -1,15 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { - MicrosoftAdaptiveDialog, - BaseSchema, - SchemaDefinitions, - SDKKinds, - SwitchCondition, -} from '@botframework-composer/types'; +import { MicrosoftAdaptiveDialog, BaseSchema, SchemaDefinitions, SDKKinds } from '@botframework-composer/types'; +import get from 'lodash/get'; -import { discoverNestedProperties } from './schemaUtils'; +import { discoverNestedPaths } from './schemaUtils'; // returns true to continue the visit. type VisitAdaptiveComponentFn = ( @@ -47,19 +42,12 @@ const walkWithPath = ( fn: VisitAdaptiveComponentFn ): boolean => { const { $kind } = adaptiveData; - - // SwitchCondition needs to be handled specially due to 'CaseCondition' doesn't conains a $kind property but is iterable. - // Reference: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Actions/Case.cs - if ($kind === SDKKinds.SwitchCondition) { - return walkSwitchCondition(adaptiveData, sdkSchema, currentPath, parentPath, fn); - } - // Visit current data before schema validation to make sure all $kind blocks are visited. fn($kind, adaptiveData, currentPath, parentPath); const schema = sdkSchema[$kind]; - const nestedProperties = schema ? discoverNestedProperties(schema) : adaptiveData.actions ? ['actions'] : []; - if (nestedProperties.length === 0) return true; + const nestedPaths = schema ? discoverNestedPaths(adaptiveData, schema) : adaptiveData.actions ? ['actions'] : []; + if (nestedPaths.length === 0) return true; /** * Examples of nested properties in built-in $kinds: @@ -68,8 +56,8 @@ const walkWithPath = ( * 3. ['actions', 'elseActions'] in IfCondition * 4. ['cases', 'default'] in SwitchCondition */ - for (const propName of nestedProperties) { - const childElements = adaptiveData[propName]; + for (const path of nestedPaths) { + const childElements = get(adaptiveData, path); if (!Array.isArray(childElements)) { continue; } @@ -84,36 +72,7 @@ const walkWithPath = ( const shouldContinue = walkWithPath( childElements[i], sdkSchema, - joinPath(currentPath, `${propName}[${i}]`), - currentPath, - fn - ); - if (!shouldContinue) return false; - } - } - - return true; -}; - -// TODO: optimize the discoverNestedProperties handler to makes cases[i].actions discoverable. -const walkSwitchCondition = ( - switchConditionData: SwitchCondition, - sdkSchema: SchemaDefinitions, - currentPath: string, - parrentPath: string, - fn: VisitAdaptiveComponentFn -): boolean => { - fn(SDKKinds.SwitchCondition, switchConditionData, currentPath, parrentPath); - - const defaultActions = switchConditionData.default; - - // Visit the 'default' properties - if (Array.isArray(defaultActions)) { - for (let i = 0; i < defaultActions.length; i++) { - const shouldContinue = walkWithPath( - defaultActions[i], - sdkSchema, - joinPath(currentPath, `default[${i}]`), + joinPath(currentPath, `${path}[${i}]`), currentPath, fn ); @@ -121,25 +80,5 @@ const walkSwitchCondition = ( } } - const casesData = switchConditionData.cases; - if (Array.isArray(casesData)) { - // Expand cases data - for (let iCase = 0; iCase < casesData.length; iCase++) { - const caseActions = casesData[iCase].actions; - if (!Array.isArray(caseActions)) continue; - - // Expand actions under a case. - for (let i = 0; i < caseActions.length; i++) { - const shouldContinue = walkWithPath( - caseActions[i], - sdkSchema, - `${currentPath}.cases[${iCase}].actions[${i}]`, - currentPath, - fn - ); - if (!shouldContinue) return false; - } - } - } return true; }; From c37c6d222bf039bfffbc6f6d7b700401536c01ee Mon Sep 17 00:00:00 2001 From: zeye Date: Tue, 6 Apr 2021 15:31:39 +0800 Subject: [PATCH 09/29] fix lint error: use BaseSchema --- .../indexers/src/validations/schemaValidation/index.ts | 8 ++------ .../validations/schemaValidation/walkAdaptiveDialog.ts | 4 ++-- Composer/packages/types/src/schema.ts | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts index f6b7bd2889..2bf294856b 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/index.ts @@ -2,17 +2,13 @@ // Licensed under the MIT License. import { Diagnostic } from '@bfc/shared'; import formatMessage from 'format-message'; -import { DiagnosticSeverity, MicrosoftAdaptiveDialog, SchemaDefinitions } from '@botframework-composer/types'; +import { BaseSchema, DiagnosticSeverity, SchemaDefinitions } from '@botframework-composer/types'; import { walkAdaptiveDialog } from './walkAdaptiveDialog'; const SCHEMA_NOT_FOUND = formatMessage('Schema definition not found in sdk.schema.'); -export const validateSchema = ( - dialogId: string, - dialogData: MicrosoftAdaptiveDialog, - schema: SchemaDefinitions -): Diagnostic[] => { +export const validateSchema = (dialogId: string, dialogData: BaseSchema, schema: SchemaDefinitions): Diagnostic[] => { const diagnostics: Diagnostic[] = []; const schemas: any = schema.definitions ?? {}; diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts index e2ee4625c1..1b7d89e19f 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/walkAdaptiveDialog.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { MicrosoftAdaptiveDialog, BaseSchema, SchemaDefinitions, SDKKinds } from '@botframework-composer/types'; +import { BaseSchema, SchemaDefinitions, SDKKinds } from '@botframework-composer/types'; import get from 'lodash/get'; import { discoverNestedPaths } from './schemaUtils'; @@ -15,7 +15,7 @@ type VisitAdaptiveComponentFn = ( ) => boolean; export const walkAdaptiveDialog = ( - adaptiveDialog: MicrosoftAdaptiveDialog, + adaptiveDialog: BaseSchema, sdkSchema: SchemaDefinitions, fn: VisitAdaptiveComponentFn ): boolean => { diff --git a/Composer/packages/types/src/schema.ts b/Composer/packages/types/src/schema.ts index eb9ffc7b0d..08699b4466 100644 --- a/Composer/packages/types/src/schema.ts +++ b/Composer/packages/types/src/schema.ts @@ -149,7 +149,7 @@ interface AdaptiveSchema extends Omit Date: Tue, 6 Apr 2021 17:23:34 +0800 Subject: [PATCH 10/29] feat: disable actions without schema --- .../DiagnosticsTab/DiagnosticsTabHeader.tsx | 2 ++ .../DiagnosticsTab/useAutoFix.ts | 36 +++++++++++++++++++ .../client/src/pages/diagnostics/types.ts | 5 +++ .../selectors/diagnosticsPageSelector.ts | 3 +- 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/DiagnosticsTabHeader.tsx b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/DiagnosticsTabHeader.tsx index 3415f3d786..e75c51ed1e 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/DiagnosticsTabHeader.tsx +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/DiagnosticsTabHeader.tsx @@ -5,10 +5,12 @@ import { jsx, css } from '@emotion/core'; import formatMessage from 'format-message'; +import { useAutoFix } from './useAutoFix'; import { useDiagnosticsStatistics } from './useDiagnostics'; export const DiagnosticsHeader = () => { const { hasError, hasWarning } = useDiagnosticsStatistics(); + useAutoFix(); return (
{ + const diagnostics = useDiagnosticsData(); + const botProjectSpace = useRecoilValue(botProjectSpaceSelector); + const { updateDialog } = useRecoilValue(dispatcherState); + + // Auto fix schema absence by setting 'disabled' to true. + const schemaDiagnostics = diagnostics.filter((d) => d.type === DiagnosticType.SCHEMA) as SchemaDiagnostic[]; + schemaDiagnostics.forEach((d) => { + const { projectId, rootProjectId, id: dialogId, dialogPath } = d; + + const dialogContent = botProjectSpace + .find((bot) => bot.projectId === projectId) + ?.dialogs.find((dialog) => dialog.id === dialogId)?.content; + if (!dialogContent) return; + + if (get(dialogContent, `${dialogPath}.disabled`)) return; + + const copy = cloneDeep(dialogContent); + set(copy, `${dialogPath}.disabled`, true); + console.log('Autofix: disable', projectId, rootProjectId, dialogPath); + updateDialog({ id: dialogId, projectId, content: copy }); + }); +}; diff --git a/Composer/packages/client/src/pages/diagnostics/types.ts b/Composer/packages/client/src/pages/diagnostics/types.ts index 82a026e5f3..4073e8c33e 100644 --- a/Composer/packages/client/src/pages/diagnostics/types.ts +++ b/Composer/packages/client/src/pages/diagnostics/types.ts @@ -17,6 +17,7 @@ export enum DiagnosticType { SKILL, SETTING, GENERAL, + SCHEMA, } export interface IDiagnosticInfo { @@ -94,6 +95,10 @@ export class DialogDiagnostic extends DiagnosticInfo { }; } +export class SchemaDiagnostic extends DialogDiagnostic { + type = DiagnosticType.SCHEMA; +} + export class SkillSettingDiagnostic extends DiagnosticInfo { type = DiagnosticType.SKILL; constructor(rootProjectId: string, projectId: string, id: string, location: string, diagnostic: Diagnostic) { diff --git a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts index 879f882736..144964d77b 100644 --- a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts +++ b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts @@ -18,6 +18,7 @@ import { BotDiagnostic, SettingDiagnostic, SkillSettingDiagnostic, + SchemaDiagnostic, } from '../../pages/diagnostics/types'; import { botDiagnosticsState, @@ -196,7 +197,7 @@ export const schemaDiagnosticsSelectorFamily = selectorFamily({ botAssets.dialogs.forEach((dialog) => { const diagnostics = validateSchema(dialog.id, dialog.content, sdkSchemaContent); fullDiagnostics.push( - ...diagnostics.map((d) => new DialogDiagnostic(rootProjectId, projectId, dialog.id, `${dialog.id}.dialog`, d)) + ...diagnostics.map((d) => new SchemaDiagnostic(rootProjectId, projectId, dialog.id, `${dialog.id}.dialog`, d)) ); }); return fullDiagnostics; From f48e74f96a3b183b44777b91950bf0a83a5e62d4 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Wed, 7 Apr 2021 19:51:16 +0800 Subject: [PATCH 11/29] wrap in useEffect --- .../DiagnosticsTab/useAutoFix.ts | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts index c31acc6ade..dfc2e33bc5 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts @@ -5,6 +5,7 @@ import get from 'lodash/get'; import set from 'lodash/set'; import cloneDeep from 'lodash/cloneDeep'; import { useRecoilValue } from 'recoil'; +import { useEffect } from 'react'; import { DiagnosticType, SchemaDiagnostic } from '../../../../diagnostics/types'; import { botProjectSpaceSelector, dispatcherState } from '../../../../../recoilModel'; @@ -16,21 +17,23 @@ export const useAutoFix = () => { const botProjectSpace = useRecoilValue(botProjectSpaceSelector); const { updateDialog } = useRecoilValue(dispatcherState); - // Auto fix schema absence by setting 'disabled' to true. - const schemaDiagnostics = diagnostics.filter((d) => d.type === DiagnosticType.SCHEMA) as SchemaDiagnostic[]; - schemaDiagnostics.forEach((d) => { - const { projectId, rootProjectId, id: dialogId, dialogPath } = d; - - const dialogContent = botProjectSpace - .find((bot) => bot.projectId === projectId) - ?.dialogs.find((dialog) => dialog.id === dialogId)?.content; - if (!dialogContent) return; - - if (get(dialogContent, `${dialogPath}.disabled`)) return; - - const copy = cloneDeep(dialogContent); - set(copy, `${dialogPath}.disabled`, true); - console.log('Autofix: disable', projectId, rootProjectId, dialogPath); - updateDialog({ id: dialogId, projectId, content: copy }); - }); + useEffect(() => { + // Auto fix schema absence by setting 'disabled' to true. + const schemaDiagnostics = diagnostics.filter((d) => d.type === DiagnosticType.SCHEMA) as SchemaDiagnostic[]; + schemaDiagnostics.forEach((d) => { + const { projectId, rootProjectId, id: dialogId, dialogPath } = d; + + const dialogContent = botProjectSpace + .find((bot) => bot.projectId === projectId) + ?.dialogs.find((dialog) => dialog.id === dialogId)?.content; + if (!dialogContent) return; + + if (get(dialogContent, `${dialogPath}.disabled`)) return; + + const copy = cloneDeep(dialogContent); + set(copy, `${dialogPath}.disabled`, true); + console.log('Autofix: disable', projectId, rootProjectId, dialogPath); + updateDialog({ id: dialogId, projectId, content: copy }); + }); + }, [diagnostics]); }; From aa91b42c1cca631ec4aed4a0c76b90f9284b57b5 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Wed, 7 Apr 2021 20:11:37 +0800 Subject: [PATCH 12/29] optimization: avoid frequent recoil submission --- .../DiagnosticsTab/useAutoFix.ts | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts index dfc2e33bc5..cd42065525 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts @@ -17,23 +17,44 @@ export const useAutoFix = () => { const botProjectSpace = useRecoilValue(botProjectSpaceSelector); const { updateDialog } = useRecoilValue(dispatcherState); + // Auto fix schema absence by setting 'disabled' to true. useEffect(() => { - // Auto fix schema absence by setting 'disabled' to true. + /** + * Caches updated dialogs data as a tree to avoid frequent recoil state submission. + * + * Example: + * { + * // projectId + * '2096.637': { + * // dialogId + * 'dialog-1': {...} // updated dialog json + * } + * } + */ + const cachedDialogs: { [projectId: string]: { [dialogId: string]: any } } = {}; + const schemaDiagnostics = diagnostics.filter((d) => d.type === DiagnosticType.SCHEMA) as SchemaDiagnostic[]; schemaDiagnostics.forEach((d) => { - const { projectId, rootProjectId, id: dialogId, dialogPath } = d; + const { projectId, id: dialogId, dialogPath } = d; const dialogContent = botProjectSpace .find((bot) => bot.projectId === projectId) ?.dialogs.find((dialog) => dialog.id === dialogId)?.content; - if (!dialogContent) return; + // Contains two cases: 1. action already disabled 2. action doesn't exists on this path. if (get(dialogContent, `${dialogPath}.disabled`)) return; - const copy = cloneDeep(dialogContent); - set(copy, `${dialogPath}.disabled`, true); - console.log('Autofix: disable', projectId, rootProjectId, dialogPath); - updateDialog({ id: dialogId, projectId, content: copy }); + // Manipulate the dialog content and caches the result + const dialogData = get(cachedDialogs, [projectId, dialogId]) ?? cloneDeep(dialogContent); + set(dialogData, `${dialogPath}.disabled`, true); + set(cachedDialogs, [projectId, dialogId], dialogData); }); + + // Submit cached dialog updates to recoil store. + for (const [projectId, dialogs] of Object.entries(cachedDialogs)) { + for (const [dialogId, dialogData] of Object.entries(dialogs)) { + updateDialog({ id: dialogId, projectId, content: dialogData }); + } + } }, [diagnostics]); }; From d530195eff343a3f3e66756041352feba279c426 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Wed, 7 Apr 2021 20:38:43 +0800 Subject: [PATCH 13/29] optimization: aggregate paths rather than updatedDialog to reduce time complexity --- .../DiagnosticsTab/useAutoFix.ts | 53 ++++++++++++------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts index cd42065525..b1039ef6a8 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts @@ -9,6 +9,7 @@ import { useEffect } from 'react'; import { DiagnosticType, SchemaDiagnostic } from '../../../../diagnostics/types'; import { botProjectSpaceSelector, dispatcherState } from '../../../../../recoilModel'; +import { dialogsDispatcher } from '../../../../../recoilModel/dispatchers/dialogs'; import { useDiagnosticsData } from './useDiagnostics'; @@ -19,41 +20,55 @@ export const useAutoFix = () => { // Auto fix schema absence by setting 'disabled' to true. useEffect(() => { + const schemaDiagnostics = diagnostics.filter((d) => d.type === DiagnosticType.SCHEMA) as SchemaDiagnostic[]; /** - * Caches updated dialogs data as a tree to avoid frequent recoil state submission. + * Aggregated diagnostic paths where contains schema problem * * Example: * { * // projectId * '2096.637': { * // dialogId - * 'dialog-1': {...} // updated dialog json + * 'dialog-1': [ + * 'triggers[0].actions[1]', // diagnostics.dialogPath + * 'triggers[2].actions[3]' + * ] * } * } */ - const cachedDialogs: { [projectId: string]: { [dialogId: string]: any } } = {}; - - const schemaDiagnostics = diagnostics.filter((d) => d.type === DiagnosticType.SCHEMA) as SchemaDiagnostic[]; + const aggregatedPaths: { [projectId: string]: { [dialogId: string]: string[] } } = {}; schemaDiagnostics.forEach((d) => { const { projectId, id: dialogId, dialogPath } = d; + if (!dialogPath) return; + const currentPaths = get(aggregatedPaths, [projectId, dialogId]); + if (currentPaths) { + currentPaths.push(dialogPath); + } else { + set(aggregatedPaths, [projectId, dialogId], [dialogPath]); + } + }); - const dialogContent = botProjectSpace - .find((bot) => bot.projectId === projectId) - ?.dialogs.find((dialog) => dialog.id === dialogId)?.content; + // Manipulate dialog contnent + for (const [projectId, pathsByDialogId] of Object.entries(aggregatedPaths)) { + const projectDialogs = botProjectSpace.find((bot) => bot.projectId === projectId)?.dialogs; + if (!Array.isArray(projectDialogs)) continue; - // Contains two cases: 1. action already disabled 2. action doesn't exists on this path. - if (get(dialogContent, `${dialogPath}.disabled`)) return; + for (const [dialogId, paths] of Object.entries(pathsByDialogId)) { + const dialogData = projectDialogs.find((dialog) => dialog.id === dialogId)?.content; + if (!dialogData) continue; - // Manipulate the dialog content and caches the result - const dialogData = get(cachedDialogs, [projectId, dialogId]) ?? cloneDeep(dialogContent); - set(dialogData, `${dialogPath}.disabled`, true); - set(cachedDialogs, [projectId, dialogId], dialogData); - }); + const pathsToUpdate = paths.filter((p) => { + // Filters out those paths where action exists and action.disabled !== true + const data = get(dialogData, p); + return data && !get(data, 'disabled'); + }); + if (!pathsToUpdate.length) continue; - // Submit cached dialog updates to recoil store. - for (const [projectId, dialogs] of Object.entries(cachedDialogs)) { - for (const [dialogId, dialogData] of Object.entries(dialogs)) { - updateDialog({ id: dialogId, projectId, content: dialogData }); + const copy = cloneDeep(dialogData); + for (const p of pathsToUpdate) { + set(copy, `${p}.disabled`, true); + } + updateDialog({ id: dialogId, projectId, content: copy }); } } }, [diagnostics]); From 8081a078f2d2998a3c5da2f4ca1f7c93df813d89 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Wed, 7 Apr 2021 20:42:52 +0800 Subject: [PATCH 14/29] chore: comments & var name --- .../TabExtensions/DiagnosticsTab/useAutoFix.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts index b1039ef6a8..58641d207b 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts @@ -37,6 +37,7 @@ export const useAutoFix = () => { * } */ const aggregatedPaths: { [projectId: string]: { [dialogId: string]: string[] } } = {}; + schemaDiagnostics.forEach((d) => { const { projectId, id: dialogId, dialogPath } = d; if (!dialogPath) return; @@ -48,22 +49,24 @@ export const useAutoFix = () => { } }); - // Manipulate dialog contnent for (const [projectId, pathsByDialogId] of Object.entries(aggregatedPaths)) { - const projectDialogs = botProjectSpace.find((bot) => bot.projectId === projectId)?.dialogs; - if (!Array.isArray(projectDialogs)) continue; + // Locates dialogs in current project + const dialogsInProject = botProjectSpace.find((bot) => bot.projectId === projectId)?.dialogs; + if (!Array.isArray(dialogsInProject)) continue; for (const [dialogId, paths] of Object.entries(pathsByDialogId)) { - const dialogData = projectDialogs.find((dialog) => dialog.id === dialogId)?.content; + // Queries out current dialog data + const dialogData = dialogsInProject.find((dialog) => dialog.id === dialogId)?.content; if (!dialogData) continue; + // Filters out those paths where action exists and action.disabled !== true const pathsToUpdate = paths.filter((p) => { - // Filters out those paths where action exists and action.disabled !== true const data = get(dialogData, p); return data && !get(data, 'disabled'); }); if (!pathsToUpdate.length) continue; + // Manipulates the 'disabled' property and then submit to Recoil store. const copy = cloneDeep(dialogData); for (const p of pathsToUpdate) { set(copy, `${p}.disabled`, true); From a27fe705b3f2b57db49dc1e9ccb4a09b574ef970 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Wed, 7 Apr 2021 20:48:19 +0800 Subject: [PATCH 15/29] lint --- .../design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts index 58641d207b..d9314fab26 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts @@ -9,7 +9,6 @@ import { useEffect } from 'react'; import { DiagnosticType, SchemaDiagnostic } from '../../../../diagnostics/types'; import { botProjectSpaceSelector, dispatcherState } from '../../../../../recoilModel'; -import { dialogsDispatcher } from '../../../../../recoilModel/dispatchers/dialogs'; import { useDiagnosticsData } from './useDiagnostics'; From bc9fe60fcaa65cb21aca424a37823d6ef7f9f92b Mon Sep 17 00:00:00 2001 From: yeze322 Date: Wed, 7 Apr 2021 20:52:20 +0800 Subject: [PATCH 16/29] add comments --- .../DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts index d9314fab26..5766968836 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/DiagnosticsTab/useAutoFix.ts @@ -37,6 +37,7 @@ export const useAutoFix = () => { */ const aggregatedPaths: { [projectId: string]: { [dialogId: string]: string[] } } = {}; + // Aggregates schema diagnostics by projectId, dialogId schemaDiagnostics.forEach((d) => { const { projectId, id: dialogId, dialogPath } = d; if (!dialogPath) return; @@ -48,6 +49,7 @@ export const useAutoFix = () => { } }); + // Consumes aggregatedPaths to update dialogs in recoil store for (const [projectId, pathsByDialogId] of Object.entries(aggregatedPaths)) { // Locates dialogs in current project const dialogsInProject = botProjectSpace.find((bot) => bot.projectId === projectId)?.dialogs; From 9bdb6451c46b3815860093c9047b3a6dd24e7181 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Thu, 15 Apr 2021 12:43:16 +0800 Subject: [PATCH 17/29] defense undefined skip-level 'actions' --- .../design/DebugPanel/TabExtensions/index.ts | 2 +- .../__mocks__/sdkSchemaMocks.ts | 57 +++++++++++++++++++ .../schemaValidation/schemaUtils.test.ts | 15 +++++ .../schemaValidation/schemaUtils.ts | 21 ++++++- 4 files changed, 92 insertions(+), 3 deletions(-) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts index f17f2ca037..8d33f1ae8c 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts @@ -3,7 +3,7 @@ import { TabExtensionConfig } from './types'; import { DiagnosticsTabConfig } from './DiagnosticsTab'; -import { WebChatLogTabConfig } from './WebChatLog/config'; +import { WebChatLogTabConfig } from './WebchatLog/config'; import { RuntimeOutputTabConfig } from './RuntimeOutputLog'; const implementedDebugExtensions: TabExtensionConfig[] = [ diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts index ee8b7dd371..e13b92d786 100644 --- a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/__mocks__/sdkSchemaMocks.ts @@ -210,3 +210,60 @@ export const SwitchConditionSchema: JSONSchema7 = { }, }, }; + +export const SetPropertiesSchema: JSONSchema7 = { + "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema", + "$role": "implements(Microsoft.IDialog)", + "title": "Set property", + "description": "Set one or more property values.", + "type": "object", + "required": [ + "assignments" + ], + "properties": { + "id": { + "type": "string", + "title": "Id", + "description": "Optional id for the dialog" + }, + "disabled": { + "$ref": "schema:#/definitions/booleanExpression", + "title": "Disabled", + "description": "Optional condition which if true will disable this action.", + "examples": [ + true, + "=user.age > 3" + ] + }, + "assignments": { + "type": "array", + "title": "Assignments", + "description": "Property value assignments to set.", + "items": { + "type": "object", + "title": "Assignment", + "description": "Property assignment.", + "properties": { + "property": { + "$ref": "schema:#/definitions/stringExpression", + "title": "Property", + "description": "Property (named location to store information).", + "examples": [ + "user.age" + ] + }, + "value": { + "$ref": "schema:#/definitions/valueExpression", + "title": "Value", + "description": "New value or expression.", + "examples": [ + "='milk'", + "=dialog.favColor", + "=dialog.favColor == 'red'" + ] + } + } + } + } + } +} diff --git a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts index 6c9c501388..8020e3eaa1 100644 --- a/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts +++ b/Composer/packages/lib/indexers/__tests__/validations/schemaValidation/schemaUtils.test.ts @@ -9,6 +9,7 @@ import { IfConditionSchema, OnConvUpdateSchema, OnDialogEventSchema, + SetPropertiesSchema, SwitchConditionSchema, } from './__mocks__/sdkSchemaMocks'; @@ -30,4 +31,18 @@ describe('#schemaUtils', () => { expect.arrayContaining(['cases[0].actions', 'default']) ); }); + + it('disconverNestedProperties() should defense invalid input.', () => { + const setPropertiesStub = { + $kind: 'Microsoft.SetProperties', + $designer: { + id: 'sJzdQm', + }, + assignments: [ + { property: 'username', value: 'test' } + ] + }; + + expect(discoverNestedPaths(setPropertiesStub, SetPropertiesSchema)).toEqual([]); + }) }); diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts index 3c1ae86b22..b721e2ba65 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts @@ -15,6 +15,9 @@ export const isTrigger = (schema: JSONSchema7): boolean => { const triggerNesterProperties = ['actions']; const propertyDefinesActionArray = (propertyDefinition: JSONSchema7): boolean => { + if (!propertyDefinition) { + return false; + } const { type, items } = propertyDefinition; return type === 'array' && Boolean(get(items, '$kind')); }; @@ -71,12 +74,26 @@ export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): stri continue; } - const schemaHasSkipLevelActions = type === 'array' && propertyDefinesActionArray(get(items, 'properties.actions')); - /** * Discover skip-level child elements. Currently, this logic is for handling SwitchCondition. * Reference to SwitchCondition.schema: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Actions/Microsoft.SwitchCondition.schema + * + * Example: + * properties.cases.items.properties = { + * "value": { ... }, + * "actions": { // Discover this property + * "type": "array", + * "items": { + * "$kind": "Microsoft.IDialog" + * }, + * "title": "Actions", + * "description": "Actions to execute." + * } + * } */ + const actionsUnderItems = get(items, 'properties.actions'); + const schemaHasSkipLevelActions = type === 'array' && Boolean(actionsUnderItems) && propertyDefinesActionArray(actionsUnderItems); + if (schemaHasSkipLevelActions) { propertyData.forEach((caseData, caseIndex) => { const caseActions = caseData.actions; From 5788b6a33c00d7e55b65e7d8ad2037b9d5f8acdc Mon Sep 17 00:00:00 2001 From: yeze322 Date: Thu, 15 Apr 2021 12:53:28 +0800 Subject: [PATCH 18/29] defense potential exceptions --- .../schemaValidation/schemaUtils.ts | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts index b721e2ba65..000cc2f325 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts @@ -22,7 +22,7 @@ const propertyDefinesActionArray = (propertyDefinition: JSONSchema7): boolean => return type === 'array' && Boolean(get(items, '$kind')); }; -export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): string[] => { +const discoverNestedSchemaPaths = (data: BaseSchema, schema: JSONSchema7): string[] => { if (isTrigger(schema)) return triggerNesterProperties; if (!schema.properties) return []; @@ -31,8 +31,6 @@ export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): stri const entries = Object.entries(schema.properties); for (const entry of entries) { const [propertyName, propertyDef] = entry; - const { type, items } = propertyDef as any; - /** * Discover child elements (triggers, actions). For example: * 1. In Microsoft.IfCondition.schema @@ -75,10 +73,11 @@ export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): stri } /** - * Discover skip-level child elements. Currently, this logic is for handling SwitchCondition. - * Reference to SwitchCondition.schema: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Actions/Microsoft.SwitchCondition.schema + * Discover skip-level child elements. + * Currently, this logic can only handle skip-level child actions under the 'actions' field. + * To discover all possible actions under arbitrary levels / field names, needs to traverse the schema tree. * - * Example: + * Example: (Reference to SwitchCondition.schema: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Actions/Microsoft.SwitchCondition.schema) * properties.cases.items.properties = { * "value": { ... }, * "actions": { // Discover this property @@ -91,8 +90,8 @@ export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): stri * } * } */ - const actionsUnderItems = get(items, 'properties.actions'); - const schemaHasSkipLevelActions = type === 'array' && Boolean(actionsUnderItems) && propertyDefinesActionArray(actionsUnderItems); + const actionsDefUnderItems = get(propertyDef, 'items.properties.actions'); + const schemaHasSkipLevelActions = propertyDef?.type === 'array' && Boolean(actionsDefUnderItems) && propertyDefinesActionArray(actionsDefUnderItems); if (schemaHasSkipLevelActions) { propertyData.forEach((caseData, caseIndex) => { @@ -108,3 +107,12 @@ export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): stri return nestedPaths; }; + +export const discoverNestedPaths = (data: BaseSchema, schema: JSONSchema7): string[] => { + try { + return discoverNestedSchemaPaths(data, schema); + } catch (e) { + // Met potential schema visit bugs + return []; + } +} \ No newline at end of file From 8e534b1cef7e62563afdb2f7c97dd79d4755f750 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Thu, 15 Apr 2021 13:42:38 +0800 Subject: [PATCH 19/29] get sdk.schema content correctly --- .../src/recoilModel/selectors/diagnosticsPageSelector.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts index 144964d77b..9fc549c30d 100644 --- a/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts +++ b/Composer/packages/client/src/recoilModel/selectors/diagnosticsPageSelector.ts @@ -185,12 +185,17 @@ export const schemaDiagnosticsSelectorFamily = selectorFamily({ key: 'schemaDiagnosticsSelectorFamily', get: (projectId: string) => ({ get }) => { const botAssets = get(botAssetsSelectFamily(projectId)); - // Why botAssets.dialogSchemas is a list? if (botAssets === null) return []; const rootProjectId = get(rootBotProjectIdSelector) ?? projectId; - const sdkSchemaContent = botAssets.dialogSchemas[0]?.content; + /** + * `botAssets.dialogSchema` contains all *.schema files loaded by project indexer. However, it actually messes up sdk.schema and *.dialog.schema. + * To get the correct sdk.schema content, current workaround is to filter schema by id. + * + * TODO: To fix it entirely, we need to differentiate dialog.schema from sdk.schema in indexer. + */ + const sdkSchemaContent = botAssets.dialogSchemas.find((d) => d.id === '')?.content; if (!sdkSchemaContent) return []; const fullDiagnostics: DiagnosticInfo[] = []; From c2e461a1ed0a1a53f42d17562883bb10687ae2b1 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Thu, 15 Apr 2021 14:06:20 +0800 Subject: [PATCH 20/29] fix lint --- .../indexers/src/validations/schemaValidation/schemaUtils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts index 000cc2f325..2572566bc6 100644 --- a/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts +++ b/Composer/packages/lib/indexers/src/validations/schemaValidation/schemaUtils.ts @@ -91,7 +91,10 @@ const discoverNestedSchemaPaths = (data: BaseSchema, schema: JSONSchema7): strin * } */ const actionsDefUnderItems = get(propertyDef, 'items.properties.actions'); - const schemaHasSkipLevelActions = propertyDef?.type === 'array' && Boolean(actionsDefUnderItems) && propertyDefinesActionArray(actionsDefUnderItems); + const schemaHasSkipLevelActions = + propertyDef?.type === 'array' + && Boolean(actionsDefUnderItems) + && propertyDefinesActionArray(actionsDefUnderItems); if (schemaHasSkipLevelActions) { propertyData.forEach((caseData, caseIndex) => { From e757e1bb6bfae377a3d3565f9a61ef5fb8b983d3 Mon Sep 17 00:00:00 2001 From: yeze322 Date: Thu, 15 Apr 2021 14:59:34 +0800 Subject: [PATCH 21/29] fix folder name case problem --- .../client/src/pages/design/DebugPanel/TabExtensions/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts index 8d33f1ae8c..f17f2ca037 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/index.ts @@ -3,7 +3,7 @@ import { TabExtensionConfig } from './types'; import { DiagnosticsTabConfig } from './DiagnosticsTab'; -import { WebChatLogTabConfig } from './WebchatLog/config'; +import { WebChatLogTabConfig } from './WebChatLog/config'; import { RuntimeOutputTabConfig } from './RuntimeOutputLog'; const implementedDebugExtensions: TabExtensionConfig[] = [ From 4080b6e4d9fc9a5f088a142aa23b9e43e75196b4 Mon Sep 17 00:00:00 2001 From: Ben Brown Date: Thu, 22 Apr 2021 10:45:35 -0500 Subject: [PATCH 22/29] Do not specify the luis endpoint key as a parameter to the runtime if no vlaue is present (#7240) (leaving this paramter blank causes issues on windows) --- extensions/localPublish/src/index.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/extensions/localPublish/src/index.ts b/extensions/localPublish/src/index.ts index 3c46508dd1..8f6d673329 100644 --- a/extensions/localPublish/src/index.ts +++ b/extensions/localPublish/src/index.ts @@ -402,17 +402,15 @@ class LocalPublisher implements PublishPlugin { } config = this.getConfig(settings, skillHostEndpoint); let spawnProcess; + const args = [...commandAndArgs, '--port', port, `--urls`, `http://0.0.0.0:${port}`, ...config]; + this.composer.log('Executing command with arguments: %s %s', startCommand, args.join(' ')); try { - spawnProcess = spawn( - startCommand, - [...commandAndArgs, '--port', port, `--urls`, `http://0.0.0.0:${port}`, ...config], - { - cwd: botDir, - stdio: ['ignore', 'pipe', 'pipe'], - detached: !isWin, // detach in non-windows - shell: isWin, // run in a shell on windows so `npm start` doesn't need to be `npm.cmd start` - } - ); + spawnProcess = spawn(startCommand, args, { + cwd: botDir, + stdio: ['ignore', 'pipe', 'pipe'], + detached: !isWin, // detach in non-windows + shell: isWin, // run in a shell on windows so `npm start` doesn't need to be `npm.cmd start` + }); this.composer.log('Started process %d', spawnProcess.pid); this.setBotStatus(botId, { process: spawnProcess, @@ -455,7 +453,7 @@ class LocalPublisher implements PublishPlugin { configList.push('--MicrosoftAppPassword'); configList.push(config.MicrosoftAppPassword); } - if (config.luis) { + if (config.luis && (config.luis.endpointKey || config.luis.authoringKey)) { configList.push('--luis:endpointKey'); configList.push(config.luis.endpointKey || config.luis.authoringKey); } From 8635df9acc1373aa4cb01a421e45c64f55ef632a Mon Sep 17 00:00:00 2001 From: Ben Brown Date: Thu, 22 Apr 2021 12:02:46 -0500 Subject: [PATCH 23/29] disable telemetry calls in the provision dialog while we investigate why telemetryclient is null (#7256) Co-authored-by: Geoff Cox (Microsoft) --- .../src/components/azureProvisionDialog.tsx | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/extensions/azurePublish/src/components/azureProvisionDialog.tsx b/extensions/azurePublish/src/components/azureProvisionDialog.tsx index 6c1cc9c5ff..e789f47331 100644 --- a/extensions/azurePublish/src/components/azureProvisionDialog.tsx +++ b/extensions/azurePublish/src/components/azureProvisionDialog.tsx @@ -284,6 +284,8 @@ export const AzureProvisionDialog: React.FC = () => { } = usePublishApi(); const telemetryClient: TelemetryClient = useTelemetryClient(); + console.log('TELEMETRY CLIENT', telemetryClient); + const { setItem, getItem, clearAll } = useLocalStorage(); // set type of publish - azurePublish or azureFunctionsPublish const publishType = getType(); @@ -362,22 +364,22 @@ export const AzureProvisionDialog: React.FC = () => { setPage(page); switch (page) { case PageTypes.AddResources: - telemetryClient.track('ProvisionAddResourcesNavigate'); + // telemetryClient.track('ProvisionAddResourcesNavigate'); setTitle(DialogTitle.ADD_RESOURCES); break; case PageTypes.ChooseAction: setTitle(DialogTitle.CHOOSE_ACTION); break; case PageTypes.ConfigProvision: - telemetryClient.track('ProvisionConfigureResources'); + // telemetryClient.track('ProvisionConfigureResources'); setTitle(DialogTitle.CONFIG_RESOURCES); break; case PageTypes.EditJson: - telemetryClient.track('ProvisionEditJSON'); + // telemetryClient.track('ProvisionEditJSON'); setTitle(DialogTitle.EDIT); break; case PageTypes.ReviewResource: - telemetryClient.track('ProvisionReviewResources'); + // telemetryClient.track('ProvisionReviewResources'); setTitle(DialogTitle.REVIEW); break; } @@ -704,11 +706,11 @@ export const AzureProvisionDialog: React.FC = () => { const onSubmit = useCallback((options) => { // call back to the main Composer API to begin this process... - telemetryClient.track('ProvisionStart', { - region: options.location, - subscriptionId: options.subscription, - externalResources: options.externalResources, - }); + // telemetryClient.track('ProvisionStart', { + // region: options.location, + // subscriptionId: options.subscription, + // externalResources: options.externalResources, + // }); startProvision(options); clearAll(); @@ -981,7 +983,7 @@ export const AzureProvisionDialog: React.FC = () => { style={{ margin: '0 4px' }} text={formatMessage('Cancel')} onClick={() => { - telemetryClient.track('ProvisionCancel'); + // telemetryClient.track('ProvisionCancel'); closeDialog(); }} /> @@ -1077,7 +1079,7 @@ export const AzureProvisionDialog: React.FC = () => { text={formatMessage('Next')} onClick={() => { if (formData.creationType === 'generate') { - telemetryClient.track('ProvisionShowHandoff'); + // telemetryClient.track('ProvisionShowHandoff'); setShowHandoff(true); } else { setPageAndTitle(PageTypes.ReviewResource); @@ -1101,7 +1103,7 @@ export const AzureProvisionDialog: React.FC = () => { style={{ margin: '0 4px' }} text={formatMessage('Cancel')} onClick={() => { - telemetryClient.track('ProvisionAddResourcesCancel'); + // telemetryClient.track('ProvisionAddResourcesCancel'); closeDialog(); }} /> From 510624792b92ffcea79c6835f15ea756b157d136 Mon Sep 17 00:00:00 2001 From: Ben Brown Date: Thu, 22 Apr 2021 13:25:42 -0500 Subject: [PATCH 24/29] prefer the botName field instead of the name field when managing connections (#7262) --- .../client/src/pages/botProject/adapters/ABSChannels.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Composer/packages/client/src/pages/botProject/adapters/ABSChannels.tsx b/Composer/packages/client/src/pages/botProject/adapters/ABSChannels.tsx index c70f536a27..6a8c47d1ca 100644 --- a/Composer/packages/client/src/pages/botProject/adapters/ABSChannels.tsx +++ b/Composer/packages/client/src/pages/botProject/adapters/ABSChannels.tsx @@ -139,8 +139,8 @@ export const ABSChannels: React.FC = (props) => { const config = JSON.parse(profile.configuration); setCurrentResource({ microsoftAppId: config?.settings?.MicrosoftAppId, - resourceName: config.name, - resourceGroupName: config.resourceGroup || config.name, + resourceName: config.botName || config.name, + resourceGroupName: config.resourceGroup || config.botName || config.name, subscriptionId: config.subscriptionId, }); } From f24852f5c23416158e5b02f9588eab71b6bd4cb2 Mon Sep 17 00:00:00 2001 From: Srinaath Ravichandran Date: Thu, 22 Apr 2021 12:05:56 -0700 Subject: [PATCH 25/29] fix: Empty Webchat inspector text and Disabling items in PVA context (#7241) --- .../BotRuntimeController/ErrorCallout.tsx | 2 +- .../GetStarted/GetStartedNextSteps.tsx | 30 ++++--- .../components/GetStarted/GetStartedTask.tsx | 10 ++- .../client/src/components/GetStarted/types.ts | 1 + .../client/src/components/NavItem.tsx | 1 + .../WebChatLog/WebChatLogContent.tsx | 81 ++++++++++++++++--- 6 files changed, 100 insertions(+), 25 deletions(-) diff --git a/Composer/packages/client/src/components/BotRuntimeController/ErrorCallout.tsx b/Composer/packages/client/src/components/BotRuntimeController/ErrorCallout.tsx index 035bad5cef..0ec8db5c8b 100644 --- a/Composer/packages/client/src/components/BotRuntimeController/ErrorCallout.tsx +++ b/Composer/packages/client/src/components/BotRuntimeController/ErrorCallout.tsx @@ -33,8 +33,8 @@ const descriptionText = css` `; const descriptionLongText = css` - overflow: auto; font-size: small; + white-space: pre-wrap; `; const descriptionShow = css``; const descriptionHide = css` diff --git a/Composer/packages/client/src/components/GetStarted/GetStartedNextSteps.tsx b/Composer/packages/client/src/components/GetStarted/GetStartedNextSteps.tsx index 030a7d43f8..6b65bc9e42 100644 --- a/Composer/packages/client/src/components/GetStarted/GetStartedNextSteps.tsx +++ b/Composer/packages/client/src/components/GetStarted/GetStartedNextSteps.tsx @@ -18,6 +18,8 @@ import { dispatcherState, settingsState } from '../../recoilModel'; import { mergePropertiesManagedByRootBot } from '../../recoilModel/dispatchers/utils/project'; import { rootBotProjectIdSelector } from '../../recoilModel/selectors/project'; import { navigateTo } from '../../utils/navigation'; +import { DisableFeatureToolTip } from '../DisableFeatureToolTip'; +import { usePVACheck } from '../../hooks/usePVACheck'; import { GetStartedTask } from './GetStartedTask'; import { NextSteps } from './types'; @@ -48,6 +50,7 @@ export const GetStartedNextSteps: React.FC = (props) => { const [highlightLUIS, setHighlightLUIS] = useState(false); const [highlightQNA, setHighlightQNA] = useState(false); + const isPVABot = usePVACheck(projectId); const hideManageLuis = () => { setDisplayManageLuis(false); @@ -131,6 +134,7 @@ export const GetStartedNextSteps: React.FC = (props) => { setDisplayManageLuis(true); } }, + isDisabled: isPVABot, }); } if (props.requiresQNA) { @@ -151,6 +155,7 @@ export const GetStartedNextSteps: React.FC = (props) => { setDisplayManageQNA(true); } }, + isDisabled: isPVABot, }); } setRequiredNextSteps(newNextSteps); @@ -170,6 +175,7 @@ export const GetStartedNextSteps: React.FC = (props) => { TelemetryClient.track('GettingStartedActionClicked', { taskName: 'publishing', priority: 'recommended' }); openLink(linkToPublishProfile); }, + isDisabled: false, }); } setRecommendedNextSteps(newRecomendedSteps); @@ -185,6 +191,7 @@ export const GetStartedNextSteps: React.FC = (props) => { TelemetryClient.track('GettingStartedActionClicked', { taskName: 'packages', priority: 'optional' }); openLink(linkToPackageManager); }, + isDisabled: isPVABot, }, { key: 'editlg', @@ -196,6 +203,7 @@ export const GetStartedNextSteps: React.FC = (props) => { TelemetryClient.track('GettingStartedActionClicked', { taskName: 'editlg', priority: 'optional' }); openLink(linkToLGEditor); }, + isDisabled: false, }, { key: 'editlu', @@ -207,6 +215,7 @@ export const GetStartedNextSteps: React.FC = (props) => { TelemetryClient.track('GettingStartedActionClicked', { taskName: 'editlu', priority: 'optional' }); openLink(linkToLUEditor); }, + isDisabled: false, }, { key: 'insights', @@ -220,6 +229,7 @@ export const GetStartedNextSteps: React.FC = (props) => { TelemetryClient.track('GettingStartedActionClicked', { taskName: 'insights', priority: 'optional' }); openLink(linkToAppInsights); }, + isDisabled: isPVABot, }, { key: 'devops', @@ -231,6 +241,7 @@ export const GetStartedNextSteps: React.FC = (props) => { TelemetryClient.track('GettingStartedActionClicked', { taskName: 'devops', priority: 'optional' }); openLink(linkToDevOps); }, + isDisabled: false, }, ]; @@ -245,12 +256,19 @@ export const GetStartedNextSteps: React.FC = (props) => { TelemetryClient.track('GettingStartedActionClicked', { taskName: 'connections', priority: 'optional' }); openLink(linkToConnections); }, + isDisabled: isPVABot, }); } setOptionalSteps(optSteps); }, [botProject, props.requiresLUIS, props.requiresQNA, props.showTeachingBubble]); + const getStartedTaskElement = (step: NextSteps) => ( + + + + ); + return (
@@ -304,27 +322,21 @@ export const GetStartedNextSteps: React.FC = (props) => { {requiredNextSteps.length ? (

{formatMessage('Required')}

- {requiredNextSteps.map((step) => ( - - ))} + {requiredNextSteps.map((step) => getStartedTaskElement(step))}
) : null} {recommendedNextSteps.length ? (

{formatMessage('Recommended')}

- {recommendedNextSteps.map((step) => ( - - ))} + {recommendedNextSteps.map((step) => getStartedTaskElement(step))}
) : null} {optionalSteps.length ? (

{formatMessage('Optional')}

- {optionalSteps.map((step) => ( - - ))} + {optionalSteps.map((step) => getStartedTaskElement(step))}
) : null}
diff --git a/Composer/packages/client/src/components/GetStarted/GetStartedTask.tsx b/Composer/packages/client/src/components/GetStarted/GetStartedTask.tsx index 09cab3fd85..3dc21ae99b 100644 --- a/Composer/packages/client/src/components/GetStarted/GetStartedTask.tsx +++ b/Composer/packages/client/src/components/GetStarted/GetStartedTask.tsx @@ -2,7 +2,7 @@ // Licensed under the MIT License. /** @jsx jsx */ -import { jsx } from '@emotion/core'; +import { css, jsx } from '@emotion/core'; import React from 'react'; import { FluentTheme, SharedColors } from '@uifabric/fluent-theme'; import { ActionButton } from 'office-ui-fabric-react/lib/Button'; @@ -15,6 +15,12 @@ type TaskProps = { step: NextSteps; }; +const getStartedStepStyle = (disabled?: boolean) => css` + margin-bottom: 20px; + pointer-events: ${disabled ? 'none' : 'auto'}; + opacity: ${disabled ? 0.4 : 1}; +`; + export const GetStartedTask: React.FC = (props) => { const icon = props.step.checked ? 'CompletedSolid' : props.step.required ? 'Error' : 'Completed'; const color = props.step.checked @@ -23,7 +29,7 @@ export const GetStartedTask: React.FC = (props) => { ? SharedColors.orange20 : SharedColors.cyanBlue10; return ( -
+
void; highlight?: (step?: NextSteps) => void; + isDisabled: boolean; }; diff --git a/Composer/packages/client/src/components/NavItem.tsx b/Composer/packages/client/src/components/NavItem.tsx index e5d0ae5c31..88e5b9f639 100644 --- a/Composer/packages/client/src/components/NavItem.tsx +++ b/Composer/packages/client/src/components/NavItem.tsx @@ -42,6 +42,7 @@ const link = (active: boolean, disabled: boolean) => css` : `&:hover { background-color: ${NeutralColors.gray50}; } + &:focus { outline: none; .ms-Fabric--isFocusVisible &::after { diff --git a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/WebChatLog/WebChatLogContent.tsx b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/WebChatLog/WebChatLogContent.tsx index b4d7e27aec..78c514cd09 100644 --- a/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/WebChatLog/WebChatLogContent.tsx +++ b/Composer/packages/client/src/pages/design/DebugPanel/TabExtensions/WebChatLog/WebChatLogContent.tsx @@ -8,24 +8,27 @@ import { useRecoilValue } from 'recoil'; import { ConversationTrafficItem } from '@botframework-composer/types/src'; import formatMessage from 'format-message'; import debounce from 'lodash/debounce'; +import { ActionButton } from 'office-ui-fabric-react/lib/Button'; +import { FontWeights } from 'office-ui-fabric-react/lib/Styling'; +import { SharedColors } from '@uifabric/fluent-theme'; import { dispatcherState, rootBotProjectIdSelector, webChatTrafficState, webChatInspectionDataState, + botStatusState, } from '../../../../../recoilModel'; import { DebugPanelTabHeaderProps } from '../types'; import { WebChatInspectionData } from '../../../../../recoilModel/types'; +import { BotStatus } from '../../../../../constants'; +import { useBotOperations } from '../../../../../components/BotRuntimeController/useBotOperations'; +import { usePVACheck } from '../../../../../hooks/usePVACheck'; import { WebChatInspectorPane } from './WebChatInspectorPane'; import { WebChatActivityLogItem } from './WebChatActivityLogItem'; import { WebChatNetworkLogItem } from './WebChatNetworkLogItem'; -const emptyStateMessage = css` - padding-left: 16px; -`; - const logContainer = (isActive: boolean) => css` height: 100%; display: ${!isActive ? 'none' : 'flex'}; @@ -33,16 +36,30 @@ const logContainer = (isActive: boolean) => css` flex-direction: row; `; -const logPane = css` +const logPane = (trafficLength: number) => css` height: 100%; width: 100%; display: flex; overflow: auto; flex-direction: column; - padding: 16px 0; + padding: ${trafficLength ? '16px 0' : '4px 0'}; box-sizing: border-box; `; +const emptyStateMessageContainer = css` + padding: 0px 16px; + font-size: 12px; +`; + +const actionButton = { + root: { + fontSize: 12, + fontWeight: FontWeights.regular, + color: SharedColors.cyanBlue10, + paddingLeft: 0, + }, +}; + const itemIsSelected = (item: ConversationTrafficItem, currentInspectionData?: WebChatInspectionData) => { return item.id === currentInspectionData?.item?.id; }; @@ -55,7 +72,10 @@ export const WebChatLogContent: React.FC = ({ isActive const [navigateToLatestEntry, navigateToLatestEntryWhenActive] = useState(false); const [currentLogItemCount, setLogItemCount] = useState(0); const webChatContainerRef = useRef(null); - const { setWebChatInspectionData } = useRecoilValue(dispatcherState); + const { setWebChatInspectionData, setWebChatPanelVisibility } = useRecoilValue(dispatcherState); + const currentStatus = useRecoilValue(botStatusState(currentProjectId ?? '')); + const { startAllBots } = useBotOperations(); + const isPVABot = usePVACheck(currentProjectId ?? ''); const navigateToNewestLogEntry = () => { if (currentLogItemCount && webChatContainerRef?.current) { @@ -160,14 +180,49 @@ export const WebChatLogContent: React.FC = ({ isActive } }; + const onOpenWebChatPanelClick = () => { + setWebChatPanelVisibility(true); + }; + + const noWebChatTrafficSection = useMemo(() => { + if (isPVABot) { + return null; + } + + if (currentStatus === BotStatus.inactive) { + return ( +
+ {formatMessage.rich('Your bot project is not running. Start your bot', { + actionButton: ({ children }) => ( + + {children} + + ), + })} +
+ ); + } + + if (currentStatus === BotStatus.connected) { + return ( +
+ {formatMessage.rich('Your bot project is running. Test in Web Chat', { + actionButton: ({ children }) => ( + + {children} + + ), + })} +
+ ); + } + return null; + }, [currentStatus]); + return (
-
- {displayedTraffic.length ? ( - displayedTraffic - ) : ( - {formatMessage('No Web Chat activity yet.')} - )} +
+ {displayedTraffic.length ? displayedTraffic : noWebChatTrafficSection}
From aa9305e1b3a3b63cb0b55670bf9bdc222abb7893 Mon Sep 17 00:00:00 2001 From: Soroush Date: Thu, 22 Apr 2021 13:04:10 -0700 Subject: [PATCH 26/29] show floating notifications over eveything (#7269) Co-authored-by: Soroush --- .../src/components/GetStarted/GetStarted.tsx | 39 +++++++------------ .../packages/client/src/components/Header.tsx | 35 +++++++++-------- .../Notifications/NotificationContainer.tsx | 35 ++++++++++------- .../Notifications/NotificationPanel.tsx | 4 +- .../packages/client/src/utils/zIndices.ts | 13 +++++++ 5 files changed, 69 insertions(+), 57 deletions(-) create mode 100644 Composer/packages/client/src/utils/zIndices.ts diff --git a/Composer/packages/client/src/components/GetStarted/GetStarted.tsx b/Composer/packages/client/src/components/GetStarted/GetStarted.tsx index 08a1309e52..0c1aa2cd8d 100644 --- a/Composer/packages/client/src/components/GetStarted/GetStarted.tsx +++ b/Composer/packages/client/src/components/GetStarted/GetStarted.tsx @@ -5,8 +5,9 @@ import { jsx } from '@emotion/core'; import React from 'react'; import formatMessage from 'format-message'; -import { Panel, IPanelStyles } from 'office-ui-fabric-react/lib/Panel'; -import { Pivot, PivotItem, IPivotStyles } from 'office-ui-fabric-react/lib/Pivot'; +import { Panel, IPanelStyles, PanelType } from 'office-ui-fabric-react/lib/Panel'; +import { Pivot, PivotItem } from 'office-ui-fabric-react/lib/Pivot'; +import { Stack } from 'office-ui-fabric-react/lib/Stack'; import { GetStartedNextSteps } from './GetStartedNextSteps'; import { GetStartedLearn } from './GetStartedLearn'; @@ -25,45 +26,35 @@ const panelStyles = { root: { marginTop: 50, }, - navigation: { - display: 'block', - height: 'auto', - }, } as IPanelStyles; -const pivotStyles = { root: { paddingLeft: 20, paddingTop: 10, width: '100%' } } as IPivotStyles; - export const GetStarted: React.FC = (props) => { const { projectId, onDismiss } = props; const renderTabs = () => { return ( - - - - - - - - + + + + + + + + + + ); }; - const onRenderNavigationContent = React.useCallback( - (props, defaultRender) => ( -
{defaultRender(props)}
- ), - [] - ); - return ( ); }; diff --git a/Composer/packages/client/src/components/Header.tsx b/Composer/packages/client/src/components/Header.tsx index 2f4b4ccfd6..39a15d084e 100644 --- a/Composer/packages/client/src/components/Header.tsx +++ b/Composer/packages/client/src/components/Header.tsx @@ -158,7 +158,7 @@ export const Header = () => { const locale = useRecoilValue(localeState(projectId)); const appUpdate = useRecoilValue(appUpdateState); const [teachingBubbleVisibility, setTeachingBubbleVisibility] = useState(); - const [showGetStartedTeachingBubble, setshowGetStartedTeachingBubble] = useState(false); + const [showGetStartedTeachingBubble, setShowGetStartedTeachingBubble] = useState(false); const settings = useRecoilValue(settingsState(projectId)); const isWebChatPanelVisible = useRecoilValue(isWebChatPanelVisibleState); const botProjectSolutionLoaded = useRecoilValue(botProjectSpaceLoadedState); @@ -213,10 +213,10 @@ export const Header = () => { // pop out get started if #getstarted is in the URL useEffect(() => { if (location.hash === '#getstarted') { - setshowGetStartedTeachingBubble(true); + setShowGetStartedTeachingBubble(true); setShowGetStarted(true); } else { - setshowGetStartedTeachingBubble(false); + setShowGetStartedTeachingBubble(false); } }, [location]); @@ -386,7 +386,7 @@ export const Header = () => { { isWebChatPanelVisible={isWebChatPanelVisible} /> ) : null} - { - setShowTeachingBubble(true); - }} - onDismiss={() => { - toggleGetStarted(false); - }} - /> + + { + setShowTeachingBubble(true); + }} + onDismiss={() => { + toggleGetStarted(false); + }} + />
); }; diff --git a/Composer/packages/client/src/components/Notifications/NotificationContainer.tsx b/Composer/packages/client/src/components/Notifications/NotificationContainer.tsx index 756176c364..9f7c8dbf87 100644 --- a/Composer/packages/client/src/components/Notifications/NotificationContainer.tsx +++ b/Composer/packages/client/src/components/Notifications/NotificationContainer.tsx @@ -4,10 +4,12 @@ /** @jsx jsx */ import { jsx, css } from '@emotion/core'; import isEmpty from 'lodash/isEmpty'; +import { Layer } from 'office-ui-fabric-react/lib/Layer'; import { useRecoilValue } from 'recoil'; import { dispatcherState } from '../../recoilModel'; import { notificationsSelector } from '../../recoilModel/selectors/notifications'; +import { zIndices } from '../../utils/zIndices'; import { NotificationCard } from './NotificationCard'; @@ -15,12 +17,15 @@ import { NotificationCard } from './NotificationCard'; const container = css` cursor: default; + top: 50px; + height: calc(100vh - 50px); position: absolute; right: 0px; padding: 6px; - z-index: 1; `; +const layerStyles = { root: { zIndex: zIndices.notificationContainer } }; + // -------------------- NotificationContainer -------------------- // export const NotificationContainer = () => { @@ -30,18 +35,20 @@ export const NotificationContainer = () => { if (isEmpty(notifications)) return null; return ( -
- {notifications.map((item) => { - return ( - - ); - })} -
+ +
+ {notifications.map((item) => { + return ( + + ); + })} +
+ ); }; diff --git a/Composer/packages/client/src/components/Notifications/NotificationPanel.tsx b/Composer/packages/client/src/components/Notifications/NotificationPanel.tsx index 9db19059aa..a86bb3c4f1 100644 --- a/Composer/packages/client/src/components/Notifications/NotificationPanel.tsx +++ b/Composer/packages/client/src/components/Notifications/NotificationPanel.tsx @@ -57,7 +57,7 @@ const NotificationPanel: React.FC = ({ {formatMessage('Clear all')} - {defaultRender!(props)} + {defaultRender?.(props)}
), [handleClearAll] @@ -67,7 +67,7 @@ const NotificationPanel: React.FC = ({ Date: Thu, 22 Apr 2021 13:37:10 -0700 Subject: [PATCH 27/29] Region for Microsoft Bot Channels Registration is now global (#7270) Co-authored-by: Ben Brown Co-authored-by: Soroush --- .../src/components/azureProvisionDialog.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/extensions/azurePublish/src/components/azureProvisionDialog.tsx b/extensions/azurePublish/src/components/azureProvisionDialog.tsx index e789f47331..b4a5a76e53 100644 --- a/extensions/azurePublish/src/components/azureProvisionDialog.tsx +++ b/extensions/azurePublish/src/components/azureProvisionDialog.tsx @@ -171,6 +171,17 @@ const onRenderLabel = (props) => { ); }; +const getResourceRegion = (item: ResourcesItem): string => { + const { key, region } = item; + switch (key) { + case AzureResourceTypes.APP_REGISTRATION: + case AzureResourceTypes.BOT_REGISTRATION: + return 'global'; + default: + return region; + } +}; + const reviewCols: IColumn[] = [ { key: 'Icon', @@ -235,7 +246,7 @@ const reviewCols: IColumn[] = [ onRender: (item: ResourcesItem) => { return (
- {item.key === AzureResourceTypes.APP_REGISTRATION ? 'global' : item?.region} + {getResourceRegion(item)}
); }, From e878aab1788ffed70dfa9de765cac50a4e735f6f Mon Sep 17 00:00:00 2001 From: Ben Brown Date: Thu, 22 Apr 2021 15:38:58 -0500 Subject: [PATCH 28/29] fix: adjust package manager feeds (#7243) * Fix #7092: set default page size to 100 items * Fixes #6854: merge community feeds into main feed, sort by downloads * Fixes #7043: include any component tagged msbot-component * improve error handling * restore different checks for declarative only vs code driven components * refactor to use includes instead of indexOf Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com> --- .../src/node/feeds/npm/npmFeed.ts | 2 ++ .../src/node/feeds/nuget/nugetFeed.ts | 2 ++ .../src/node/feeds/nuget/nugetInterfaces.ts | 1 + extensions/packageManager/src/node/index.ts | 33 ++----------------- .../packageManager/src/pages/Library.tsx | 16 +++++---- 5 files changed, 17 insertions(+), 37 deletions(-) diff --git a/extensions/packageManager/src/node/feeds/npm/npmFeed.ts b/extensions/packageManager/src/node/feeds/npm/npmFeed.ts index fd88d9d0bd..67f9aa0b78 100644 --- a/extensions/packageManager/src/node/feeds/npm/npmFeed.ts +++ b/extensions/packageManager/src/node/feeds/npm/npmFeed.ts @@ -100,6 +100,8 @@ export class NpmFeed implements IFeed { url = `${url}&from=${query.skip}`; } + url = `${url}&popularity=1.0`; + return url; } } diff --git a/extensions/packageManager/src/node/feeds/nuget/nugetFeed.ts b/extensions/packageManager/src/node/feeds/nuget/nugetFeed.ts index 87f41627e5..40fa802971 100644 --- a/extensions/packageManager/src/node/feeds/nuget/nugetFeed.ts +++ b/extensions/packageManager/src/node/feeds/nuget/nugetFeed.ts @@ -70,6 +70,8 @@ export class NuGetFeed implements IFeed { } const searchResult = httpResponse.data as INuGetSearchResult; + // sort these results by total downloads + searchResult.data = searchResult.data.sort((a, b) => b.totalDownloads - a.totalDownloads); if (searchResult.data) { return this.asPackageDefinition(searchResult); } diff --git a/extensions/packageManager/src/node/feeds/nuget/nugetInterfaces.ts b/extensions/packageManager/src/node/feeds/nuget/nugetInterfaces.ts index 32bb1c4bd1..adccbe1378 100644 --- a/extensions/packageManager/src/node/feeds/nuget/nugetInterfaces.ts +++ b/extensions/packageManager/src/node/feeds/nuget/nugetInterfaces.ts @@ -22,6 +22,7 @@ export interface INuGetPackage { versions: INuGetVersion[]; tags?: string | string[]; projectUrl?: string; + totalDownloads: number; } /** diff --git a/extensions/packageManager/src/node/index.ts b/extensions/packageManager/src/node/index.ts index 8ce2ad215a..dac4bc784a 100644 --- a/extensions/packageManager/src/node/index.ts +++ b/extensions/packageManager/src/node/index.ts @@ -15,13 +15,7 @@ import { FeedFactory } from './feeds/feedFactory'; const API_ROOT = '/api'; const hasSchema = (c) => { - // NOTE: A special case for orchestrator is included here because it does not directly include the schema - // the schema for orchestrator is in a dependent package - // additionally, our schemamerge command only returns the top level components found, even though - // it does properly discover and include the schema from this dependent package. - // without this special case, composer does not see orchestrator as being installed even though it is. - // in the future this should be resolved in the schemamerger library by causing the includesSchema property to be passed up to all parent libraries - return c.includesSchema || c.name.toLowerCase() === 'microsoft.bot.components.orchestrator'; + return c.includesSchema || c.keywords?.includes('msbot-component'); }; const isAdaptiveComponent = (c) => { @@ -110,18 +104,6 @@ export default async (composer: IExtensionRegistration): Promise => { text: formatMessage('nuget'), url: 'https://api.nuget.org/v3/index.json', readonly: true, - defaultQuery: { - prerelease: true, - semVerLevel: '2.0.0', - query: `microsoft.bot.components+tags:${botComponentTag}`, - }, - type: PackageSourceType.NuGet, - }, - { - key: 'nuget-community', - text: formatMessage('community packages'), - url: 'https://api.nuget.org/v3/index.json', - readonly: true, defaultQuery: { prerelease: true, semVerLevel: '2.0.0', @@ -134,17 +116,6 @@ export default async (composer: IExtensionRegistration): Promise => { text: formatMessage('npm'), url: `https://registry.npmjs.org/-/v1/search`, readonly: true, - defaultQuery: { - prerelease: true, - query: `keywords:${botComponentTag}+scope:microsoft`, - }, - type: PackageSourceType.NPM, - }, - { - key: 'npm-community', - text: formatMessage('JS community packages'), - url: `https://registry.npmjs.org/-/v1/search`, - readonly: true, defaultQuery: { prerelease: true, query: `keywords:${botComponentTag}`, @@ -227,6 +198,8 @@ export default async (composer: IExtensionRegistration): Promise => { composer.log('GETTING FEED', packageSource, packageSource.defaultQuery); + // set default page size to 100 + packageSource.defaultQuery.take = 100; const packages = await feed.getPackages(packageSource.defaultQuery); if (Array.isArray(packages)) { diff --git a/extensions/packageManager/src/pages/Library.tsx b/extensions/packageManager/src/pages/Library.tsx index 406f34552c..80bc3333fb 100644 --- a/extensions/packageManager/src/pages/Library.tsx +++ b/extensions/packageManager/src/pages/Library.tsx @@ -516,8 +516,6 @@ const Library: React.FC = () => { updateInstalledComponents(results.data.components); } else { - telemetryClient.track('PackageUninstallFailed', { package: selectedItem.name }); - throw new Error(results.data.message); } @@ -526,11 +524,15 @@ const Library: React.FC = () => { } catch (err) { telemetryClient.track('PackageUninstallFailed', { package: selectedItem.name }); - setApplicationLevelError({ - status: err.response.status, - message: err.response && err.response.data.message ? err.response.data.message : err, - summary: strings.importError, - }); + if (err.response) { + setApplicationLevelError({ + status: err.response.status, + message: err.response && err.response.data.message ? err.response.data.message : err, + summary: strings.importError, + }); + } else { + setApplicationLevelError(err); + } } setWorking(''); } From 73f5d4bfc11c6ed6e495e4eb220f66103d068a19 Mon Sep 17 00:00:00 2001 From: Ben Yackley <61990921+beyackle@users.noreply.github.com> Date: Thu, 22 Apr 2021 14:11:24 -0700 Subject: [PATCH 29/29] fix: remodel About page (#7191) * remodel About page * add SHA to version * fixes from suggestion * get info from Electron and use it for Release field Co-authored-by: Chris Whitten --- .../packages/client/src/pages/about/About.tsx | 101 ++++++++---------- .../packages/client/src/pages/about/styles.js | 45 +++----- .../client/src/pages/setting/SettingsPage.tsx | 7 -- .../packages/electron-server/src/preload.js | 4 +- .../packages/server/src/locales/en-US.json | 15 ++- 5 files changed, 67 insertions(+), 105 deletions(-) diff --git a/Composer/packages/client/src/pages/about/About.tsx b/Composer/packages/client/src/pages/about/About.tsx index 5497da03bc..f121a823eb 100644 --- a/Composer/packages/client/src/pages/about/About.tsx +++ b/Composer/packages/client/src/pages/about/About.tsx @@ -8,80 +8,69 @@ import { Link } from 'office-ui-fabric-react/lib/Link'; import formatMessage from 'format-message'; import { RouteComponentProps } from '@reach/router'; +import { isElectron } from '../../utils/electronUtil'; + import * as about from './styles'; export const About: React.FC = () => { return ( -
-
-

{formatMessage(`About`)}

-
-
{formatMessage(`Release: `) + (process.env.COMPOSER_VERSION || 'Unknown')}
-
-

- {formatMessage( - `Bot Framework Composer is a visual authoring canvas for building bots and other types of conversational application with the Microsoft Bot Framework technology stack. With Composer you will find everything you need to build a modern, state-of-the-art conversational experience.` - )} -

-

- {formatMessage( - `Bot Framework Composer enables developers and multi-disciplinary teams to build all kinds of conversational experiences, using the latest components from the Bot Framework: SDK, LG, LU, and declarative file formats, all without writing code.` - )} +

+
+
+ {formatMessage.rich( + 'Our privacy statement is located at https://go.microsoft.com/fwlink/?LinkID=824704. You can learn more about data collection and use in the help documentation and our privacy statement. Your use of the software operates as your consent to these practices.', + { + a: ({ children }) => {children}, + } + )} +
+
+ {formatMessage.rich( + '

Copyright (c) Microsoft Corporation.

MIT License

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

', + { p: ({ children }) =>

{children}

} + )} +
+
+ {formatMessage(`Release: `) + + (isElectron() + ? (window as any).appVersion + : `${process.env.COMPOSER_VERSION}-${process.env.GIT_SHA}` || 'Unknown')} +
+
+
+
{formatMessage(`SDK runtime packages`)}
+
- {formatMessage(`Learn more`)} + {process.env.SDK_PACKAGE_VERSION || 'Unknown'} -

-
-
-
-
{formatMessage(`SDK runtime packages`)}
-
- - {process.env.SDK_PACKAGE_VERSION || 'Unknown'} - -
+
+
+ + {formatMessage(`Getting Help`)} + +
+
+ - {formatMessage(`Getting Help`)} + {formatMessage(`Terms of Use`)}
-
-
- - - {formatMessage(`Terms of Use`)} - -
-
- - - {formatMessage(`Privacy`)} - -
-
); diff --git a/Composer/packages/client/src/pages/about/styles.js b/Composer/packages/client/src/pages/about/styles.js index 5b177bc492..d2849708e9 100644 --- a/Composer/packages/client/src/pages/about/styles.js +++ b/Composer/packages/client/src/pages/about/styles.js @@ -3,28 +3,11 @@ import { css } from '@emotion/core'; import { FontWeights, FontSizes } from 'office-ui-fabric-react/lib/Styling'; -export const outline = css` - display: flex; - flex-direction: column; - height: 100%; - margin: 32px 50px 0px 32px; - border: 1px solid #979797; - overflow-x: auto; -`; export const content = css` height: 100%; `; -export const title = css` - display: block; - height: 36px; - margin: 33px 0px 0px 42px; - font-size: ${FontSizes.xLarge}; - font-weight: ${FontWeights.semibold}; - line-height: 32px; -`; - export const body = css` width: auto; margin-top: 26px; @@ -32,43 +15,41 @@ export const body = css` `; export const version = css` - font-size: ${FontSizes.large}; + font-size: ${FontSizes.medium}; font-weight: ${FontWeights.regular}; line-height: 32px; `; -export const description = css` - font-size: ${FontSizes.mediumPlus}; - font-weight: ${FontWeights.regular}; - line-height: 32px; - width: 50%; - margin-top: 20px; -`; - -export const DiagnosticsText = css` +export const diagnosticsText = css` width: 50%; font-size: 24px; margin-top: 20px; `; export const smallText = css` - margin-top: 20px; - font-size: 13px; + margin: 20px 20px 20px 0; + font-size: 14px; +`; + +export const smallerText = css` + margin: 20px 20px 20px 0; + font-size: 12px; `; -export const DiagnosticsInfoText = css` + +export const diagnosticsInfoText = css` display: flex; justify-content: space-between; width: 550px; font-size: 24px; `; -export const DiagnosticsInfoTextAlignLeft = css` +export const diagnosticsInfoTextAlignLeft = css` text-align: left; font-size: ${FontSizes.mediumPlus}; font-weight: ${FontWeights.semibold}; `; -export const DiagnosticsInfo = css` +export const diagnosticsInfo = css` margin-top: 40px; `; diff --git a/Composer/packages/client/src/pages/setting/SettingsPage.tsx b/Composer/packages/client/src/pages/setting/SettingsPage.tsx index cf6ac83a79..6dbf09f2ec 100644 --- a/Composer/packages/client/src/pages/setting/SettingsPage.tsx +++ b/Composer/packages/client/src/pages/setting/SettingsPage.tsx @@ -106,12 +106,6 @@ const SettingPage: React.FC = () => { return settingLabels.appSettings; }, [location.pathname]); - const onRenderHeaderContent = () => { - return formatMessage( - 'This Page contains detailed information about your bot. For security reasons, they are hidden by default. To test your bot or publish to Azure, you may need to provide these settings' - ); - }; - return ( = () => { pageMode={'settings'} title={title} toolbarItems={[]} - onRenderHeaderContent={onRenderHeaderContent} > https://go.microsoft.com/fwlink/?LinkID=824704. You can learn more about data collection and use in the help documentation and our privacy statement. Your use of the software operates as your consent to these practices." + }, "output_5023cf84": { "message": "Output" }, + "p_copyright_c_microsoft_corporation_p_p_mit_licens_cd145fd6": { + "message": "

Copyright (c) Microsoft Corporation.

MIT License

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the \"Software\"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

" + }, "page_number_cdee4179": { "message": "Page number" }, @@ -3740,9 +3740,6 @@ "this_page_contains_detailed_information_about_your_224a2b04": { "message": "This Page contains detailed information about your bot. For security reasons, they are hidden by default. To test your bot or publish to Azure, you may need to provide these settings." }, - "this_page_contains_detailed_information_about_your_b2b3413b": { - "message": "This Page contains detailed information about your bot. For security reasons, they are hidden by default. To test your bot or publish to Azure, you may need to provide these settings" - }, "this_publishing_profile_profilename_is_no_longer_s_eee0f447": { "message": "This publishing profile ({ profileName }) is no longer supported. You are a member of multiple Azure tenants and the profile needs to have a tenant id associated with it. You can either edit the profile by adding the `tenantId` property to it''s configuration or create a new one." },