diff --git a/packages/fx-core/resource/package.nls.json b/packages/fx-core/resource/package.nls.json index f54ce7859e..10ebc5eae2 100644 --- a/packages/fx-core/resource/package.nls.json +++ b/packages/fx-core/resource/package.nls.json @@ -254,6 +254,8 @@ "core.copilotPlugin.scaffold.summary.warning.teamsManifest.mitigation": "Mitigation: update \"%s\" field in \"%s\".", "core.copilotPlugin.scaffold.summary.warning.teamsManifest.missingCardTemlate": "Missing \"%s\" in command \"%s\".", "core.copilotPlugin.scaffold.summary.warning.teamsManifest.missingCardTemlate.mitigation": " Mitigation: create Adaptive Card template in \"%s\" and then update \"%s\" field to the relative path in \"%s\".", + "core.copilotPlugin.scaffold.summary.warning.teamsManifest.missingCommandParameters": "Missing parameters in command \"%s\".", + "core.copilotPlugin.scaffold.summary.warning.teamsManifest.missingCommandParameters.mitigation": " Mitigation: add at least one parameter for command \"%s\" in \"%s\". The parameter name should match what's defined in \"%s\".", "core.createCapabilityQuestion.titleNew": "Capabilities", "core.createCapabilityQuestion.placeholder": "Select a capability", "core.createProjectQuestion.option.description.previewOnWindow": "Preview on Windows", diff --git a/packages/fx-core/src/component/generator/copilotPlugin/generator.ts b/packages/fx-core/src/component/generator/copilotPlugin/generator.ts index 441e2ccb72..6b286d53ed 100644 --- a/packages/fx-core/src/component/generator/copilotPlugin/generator.ts +++ b/packages/fx-core/src/component/generator/copilotPlugin/generator.ts @@ -31,6 +31,9 @@ import { generateScaffoldingSummary, logValidationResults, OpenAIPluginManifestHelper, + specParserGenerateResultAllSuccessTelemetryProperty, + specParserGenerateResultTelemetryEvent, + specParserGenerateResultWarningsTelemetryProperty, } from "./helper"; import { ValidationStatus, WarningType } from "../../../common/spec-parser/interfaces"; import { getLocalizedString } from "../../../common/localizeUtils"; @@ -42,6 +45,7 @@ import { isYamlSpecFile } from "../../../common/spec-parser/utils"; import { ConstantString } from "../../../common/spec-parser/constants"; import * as util from "util"; import { SpecParserError } from "../../../common/spec-parser/specParserError"; +import { isValidHttpUrl } from "../../../question/util"; const fromApiSpeccomponentName = "copilot-plugin-existing-api"; const fromApiSpecTemplateName = "copilot-plugin-existing-api"; @@ -52,6 +56,8 @@ const apiSpecYamlFileName = "openapi.yaml"; const apiSpecJsonFileName = "openapi.json"; const invalidApiSpecErrorName = "invalid-api-spec"; +const copilotPluginExistingApiSpecUrlTelemetryEvent = "copilot-plugin-existing-api-spec-url"; +const isRemoteUrlTelemetryProperty = "remote-url"; export interface CopilotPluginGeneratorResult { warnings?: Warning[]; @@ -126,6 +132,9 @@ export class CopilotPluginGenerator { if (templateRes.isErr()) return err(templateRes.error); const url = inputs[QuestionNames.ApiSpecLocation] ?? inputs.openAIPluginManifest?.api.url; + context.telemetryReporter.sendTelemetryEvent(copilotPluginExistingApiSpecUrlTelemetryEvent, { + [isRemoteUrlTelemetryProperty]: isValidHttpUrl(url).toString(), + }); // validate API spec const specParser = new SpecParser(url); @@ -195,7 +204,19 @@ export class CopilotPluginGenerator { adaptiveCardFolder ); + context.telemetryReporter.sendTelemetryEvent(specParserGenerateResultTelemetryEvent, { + [specParserGenerateResultAllSuccessTelemetryProperty]: generateResult.allSuccess.toString(), + [specParserGenerateResultWarningsTelemetryProperty]: generateResult.warnings + .map((w) => w.type.toString() + ": " + w.content) + .join(";"), + }); + if (generateResult.warnings.length > 0) { + generateResult.warnings.find((o) => { + if (o.type === WarningType.OperationOnlyContainsOptionalParam) { + o.content = ""; // We don't care content of this warning + } + }); warnings.push(...generateResult.warnings); } diff --git a/packages/fx-core/src/component/generator/copilotPlugin/helper.ts b/packages/fx-core/src/component/generator/copilotPlugin/helper.ts index f08d4039c0..49711b0682 100644 --- a/packages/fx-core/src/component/generator/copilotPlugin/helper.ts +++ b/packages/fx-core/src/component/generator/copilotPlugin/helper.ts @@ -60,6 +60,10 @@ enum OpenAIPluginManifestErrorType { ApiUrlMissing = "openai-plugin-api-url-missing", } +export const specParserGenerateResultTelemetryEvent = "spec-parser-generate-result"; +export const specParserGenerateResultAllSuccessTelemetryProperty = "all-success"; +export const specParserGenerateResultWarningsTelemetryProperty = "warnings"; + export interface ErrorResult { /** * The type of error. @@ -379,8 +383,32 @@ function validateTeamsManifestLength( ); } - // validate card + // validate command if (ManifestUtil.parseCommonProperties(teamsManifest).isCopilotPlugin) { + const optionalParamsOnlyWarnings = warnings.filter( + (o) => o.type === WarningType.OperationOnlyContainsOptionalParam + ); + + if (optionalParamsOnlyWarnings) { + for (const optionalParamsOnlyWarning of optionalParamsOnlyWarnings) { + resultWarnings.push( + getLocalizedString( + "core.copilotPlugin.scaffold.summary.warning.teamsManifest.missingCommandParameters", + optionalParamsOnlyWarning.data + ) + + getLocalizedString( + "core.copilotPlugin.scaffold.summary.warning.teamsManifest.missingCommandParameters.mitigation", + optionalParamsOnlyWarning.data, + path.join(AppPackageFolderName, ManifestTemplateFileName), + path.join( + AppPackageFolderName, + teamsManifest.composeExtensions![0].apiSpecificationFile ?? "" + ) + ) + ); + } + } + const commands = (teamsManifest.composeExtensions?.[0] as IComposeExtension).commands; for (const command of commands) { if (command.type === "query") { diff --git a/packages/fx-core/tests/component/generator/copilotPluginGenerator.test.ts b/packages/fx-core/tests/component/generator/copilotPluginGenerator.test.ts index a70b17de43..464e342992 100644 --- a/packages/fx-core/tests/component/generator/copilotPluginGenerator.test.ts +++ b/packages/fx-core/tests/component/generator/copilotPluginGenerator.test.ts @@ -145,7 +145,10 @@ describe("copilotPluginGenerator", function () { sandbox.stub(specParserUtils, "isYamlSpecFile").resolves(false); sandbox.stub(SpecParser.prototype, "generate").resolves({ allSuccess: true, - warnings: [{ type: WarningType.GenerateCardFailed, content: "test", data: "getPets" }], + warnings: [ + { type: WarningType.GenerateCardFailed, content: "test", data: "getPets" }, + { type: WarningType.OperationOnlyContainsOptionalParam, content: "test", data: "getPets" }, + ], }); sandbox.stub(Generator, "getDefaultVariables").resolves(undefined); sandbox.stub(Generator, "generateTemplate").resolves(ok(undefined)); @@ -154,10 +157,12 @@ describe("copilotPluginGenerator", function () { assert.isTrue(result.isOk()); if (result.isOk()) { - assert.isTrue(result.value.warnings!.length === 2); + assert.isTrue(result.value.warnings!.length === 3); assert.isFalse(result.value.warnings![0].content.includes("operation2")); assert.isUndefined(result.value.warnings![0].data); assert.equal(result.value.warnings![1].type, WarningType.GenerateCardFailed); + assert.equal(result.value.warnings![2].type, WarningType.OperationOnlyContainsOptionalParam); + assert.equal(result.value.warnings![2].content, ""); } }); @@ -552,4 +557,29 @@ describe("generateScaffoldingSummary", () => { assert.isTrue(res.includes("apiResponseRenderingTemplateFile")); assert.isTrue(res.includes("test")); }); + + it("warnings about command parameters", () => { + const composeExtension: IComposeExtension = { + composeExtensionType: "apiBased", + apiSpecificationFile: "testApiFile", + commands: [ + { + id: "command1", + type: "query", + title: "", + apiResponseRenderingTemplateFile: "apiResponseRenderingTemplateFile", + }, + ], + }; + const res = generateScaffoldingSummary( + [{ type: WarningType.OperationOnlyContainsOptionalParam, content: "", data: "getAll" }], + { + ...teamsManifest, + composeExtensions: [composeExtension], + }, + "path" + ); + + assert.isTrue(res.includes("testApiFile")); + }); });