Merge pull request #1837 from github/koesie10/fix-no-error-empty-repositories

Fix missing error message on repository selection

Author: koesie10

extensions/ql-vscode/src/remote-queries/repository-selection.ts

@@ -78,15 +78,21 @@ export async function getRepositorySelection(
     options,
   );
 
-  if (quickpick?.repositories?.length) {
+  if (!quickpick) {
+    // We don't need to display a warning pop-up in this case, since the user just escaped out of the operation.
+    // We set 'true' to make this a silent exception.
+    throw new UserCancellationException("No repositories selected", true);
+  }
+
+  if (quickpick.repositories?.length) {
     void extLogger.log(
       `Selected repositories: ${quickpick.repositories.join(", ")}`,
     );
     return { repositories: quickpick.repositories };
-  } else if (quickpick?.repositoryList) {
+  } else if (quickpick.repositoryList) {
     void extLogger.log(`Selected repository list: ${quickpick.repositoryList}`);
     return { repositoryLists: [quickpick.repositoryList] };
-  } else if (quickpick?.useCustomRepo) {
+  } else if (quickpick.useCustomRepo) {
     const customRepo = await getCustomRepo();
     if (customRepo === undefined) {
       // The user cancelled, do nothing.
@@ -99,7 +105,7 @@ export async function getRepositorySelection(
     }
     void extLogger.log(`Entered repository: ${customRepo}`);
     return { repositories: [customRepo] };
-  } else if (quickpick?.useAllReposOfOwner) {
+  } else if (quickpick.useAllReposOfOwner) {
     const owner = await getOwner();
     if (owner === undefined) {
       // The user cancelled, do nothing.
@@ -111,9 +117,9 @@ export async function getRepositorySelection(
     void extLogger.log(`Entered owner: ${owner}`);
     return { owners: [owner] };
   } else {
-    // We don't need to display a warning pop-up in this case, since the user just escaped out of the operation.
-    // We set 'true' to make this a silent exception.
-    throw new UserCancellationException("No repositories selected", true);
+    // This means the user has selected something, but there is nothing actually linked to this item. We want to show
+    // this to the user.
+    throw new UserCancellationException("No repositories selected", false);
   }
 }
 

extensions/ql-vscode/src/vscode-tests/no-workspace/remote-queries/repository-selection.test.ts

@@ -159,6 +159,28 @@ describe("repository selection", () => {
         expect(repoSelection.owners).toBeUndefined();
         expect(repoSelection.repositories).toEqual(["foo/bar", "foo/baz"]);
       });
+
+      it("should return an error for an empty repository list", async () => {
+        // Fake return values
+        quickPickSpy.mockResolvedValue({
+          repositories: [],
+        } as unknown as QuickPickItem);
+        getRemoteRepositoryListsSpy.mockReturnValue({
+          list1: ["foo/bar", "foo/baz"],
+          list2: [],
+        });
+
+        await expect(getRepositorySelection()).rejects.toThrow(
+          "No repositories selected",
+        );
+        await expect(getRepositorySelection()).rejects.toThrow(
+          UserCancellationException,
+        );
+        await expect(getRepositorySelection()).rejects.toHaveProperty(
+          "silent",
+          false,
+        );
+      });
     });
 
     describe("system level repo lists", () => {
@@ -243,6 +265,10 @@ describe("repository selection", () => {
         await expect(getRepositorySelection()).rejects.toThrow(
           UserCancellationException,
         );
+        await expect(getRepositorySelection()).rejects.toHaveProperty(
+          "silent",
+          true,
+        );
       });
     });
 
@@ -313,6 +339,10 @@ describe("repository selection", () => {
         await expect(getRepositorySelection()).rejects.toThrow(
           UserCancellationException,
         );
+        await expect(getRepositorySelection()).rejects.toHaveProperty(
+          "silent",
+          true,
+        );
       });
     });
 
@@ -407,5 +437,21 @@ describe("repository selection", () => {
         expect(repoSelection.repositories).toEqual(["owner3/repo3"]);
       });
     });
+
+    it("should allow the user to cancel", async () => {
+      // Fake return values
+      quickPickSpy.mockResolvedValue(undefined);
+
+      await expect(getRepositorySelection()).rejects.toThrow(
+        "No repositories selected",
+      );
+      await expect(getRepositorySelection()).rejects.toThrow(
+        UserCancellationException,
+      );
+      await expect(getRepositorySelection()).rejects.toHaveProperty(
+        "silent",
+        true,
+      );
+    });
   });
 });