refactor: show warning if API contains optional params only (#9843)

* refactor: draft

refactor: msg

refactor: telemetry

refactor: test

* refactor: test

* refactor: semicolon

* refactor: telemetry

* refactor: minor

* refactor: string

packages/fx-core/resource/package.nls.json

@@ -255,6 +255,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",

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);
       }
 

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.
@@ -395,8 +399,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") {

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, "");
     }
   });
 
@@ -566,4 +571,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"));
+  });
 });