Merge pull request #2343 from github/robertbrignull/results_filter

Add a component for filtering repositories based on their results

extensions/ql-vscode/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [UNRELEASED]
 
+- Added ability to filter repositories for a variant analysis to only those that have results [#2343](https://github.com/github/vscode-codeql/pull/2343)
 - Add new configuration option to allow downloading databases from http, non-secure servers. [#2332](https://github.com/github/vscode-codeql/pull/2332)
 
 ## 1.8.2 - 12 April 2023

extensions/ql-vscode/src/pure/variant-analysis-filter-sort.ts

@@ -3,6 +3,12 @@ import {
   RepositoryWithMetadata,
 } from "../variant-analysis/shared/repository";
 import { parseDate } from "./date";
+import { assertNever } from "./helpers-pure";
+
+export enum FilterKey {
+  All = "all",
+  WithResults = "withResults",
+}
 
 export enum SortKey {
   Name = "name",
@@ -13,6 +19,7 @@ export enum SortKey {
 
 export type RepositoriesFilterSortState = {
   searchValue: string;
+  filterKey: FilterKey;
   sortKey: SortKey;
 };
 
@@ -22,20 +29,43 @@ export type RepositoriesFilterSortStateWithIds = RepositoriesFilterSortState & {
 
 export const defaultFilterSortState: RepositoriesFilterSortState = {
   searchValue: "",
+  filterKey: FilterKey.All,
   sortKey: SortKey.Name,
 };
 
 export function matchesFilter(
-  repo: Pick<Repository, "fullName">,
+  item: FilterAndSortableResult,
   filterSortState: RepositoriesFilterSortState | undefined,
 ): boolean {
   if (!filterSortState) {
     return true;
   }
 
-  return repo.fullName
-    .toLowerCase()
-    .includes(filterSortState.searchValue.toLowerCase());
+  return (
+    matchesSearch(item.repository, filterSortState.searchValue) &&
+    matchesFilterKey(item.resultCount, filterSortState.filterKey)
+  );
+}
+
+function matchesSearch(
+  repository: SortableRepository,
+  searchValue: string,
+): boolean {
+  return repository.fullName.toLowerCase().includes(searchValue.toLowerCase());
+}
+
+function matchesFilterKey(
+  resultCount: number | undefined,
+  filterKey: FilterKey,
+): boolean {
+  switch (filterKey) {
+    case FilterKey.All:
+      return true;
+    case FilterKey.WithResults:
+      return resultCount !== undefined && resultCount > 0;
+    default:
+      assertNever(filterKey);
+  }
 }
 
 type SortableRepository = Pick<Repository, "fullName"> &
@@ -71,17 +101,22 @@ export function compareRepository(
   };
 }
 
-type SortableResult = {
+type FilterAndSortableResult = {
+  repository: SortableRepository;
+  resultCount?: number;
+};
+
+type FilterAndSortableResultWithIds = {
   repository: SortableRepository & Pick<Repository, "id">;
   resultCount?: number;
 };
 
 export function compareWithResults(
   filterSortState: RepositoriesFilterSortState | undefined,
-): (left: SortableResult, right: SortableResult) => number {
+): (left: FilterAndSortableResult, right: FilterAndSortableResult) => number {
   const fallbackSort = compareRepository(filterSortState);
 
-  return (left: SortableResult, right: SortableResult) => {
+  return (left: FilterAndSortableResult, right: FilterAndSortableResult) => {
     // Highest to lowest
     if (filterSortState?.sortKey === SortKey.ResultsCount) {
       const resultCount = (right.resultCount ?? 0) - (left.resultCount ?? 0);
@@ -95,7 +130,7 @@ export function compareWithResults(
 }
 
 export function filterAndSortRepositoriesWithResultsByName<
-  T extends SortableResult,
+  T extends FilterAndSortableResult,
 >(
   repositories: T[] | undefined,
   filterSortState: RepositoriesFilterSortState | undefined,
@@ -105,11 +140,13 @@ export function filterAndSortRepositoriesWithResultsByName<
   }
 
   return repositories
-    .filter((repo) => matchesFilter(repo.repository, filterSortState))
+    .filter((repo) => matchesFilter(repo, filterSortState))
     .sort(compareWithResults(filterSortState));
 }
 
-export function filterAndSortRepositoriesWithResults<T extends SortableResult>(
+export function filterAndSortRepositoriesWithResults<
+  T extends FilterAndSortableResultWithIds,
+>(
   repositories: T[] | undefined,
   filterSortState: RepositoriesFilterSortStateWithIds | undefined,
 ): T[] | undefined {
@@ -117,6 +154,7 @@ export function filterAndSortRepositoriesWithResults<T extends SortableResult>(
     return undefined;
   }
 
+  // If repository IDs are given, then ignore the search value and filter key
   if (
     filterSortState?.repositoryIds &&
     filterSortState.repositoryIds.length > 0

extensions/ql-vscode/src/stories/variant-analysis/RepositoriesFilter.stories.tsx

@@ -0,0 +1,25 @@
+import * as React from "react";
+import { useState } from "react";
+
+import { ComponentMeta } from "@storybook/react";
+
+import { RepositoriesFilter as RepositoriesFilterComponent } from "../../view/variant-analysis/RepositoriesFilter";
+import { FilterKey } from "../../pure/variant-analysis-filter-sort";
+
+export default {
+  title: "Variant Analysis/Repositories Filter",
+  component: RepositoriesFilterComponent,
+  argTypes: {
+    value: {
+      control: {
+        disable: true,
+      },
+    },
+  },
+} as ComponentMeta<typeof RepositoriesFilterComponent>;
+
+export const RepositoriesFilter = () => {
+  const [value, setValue] = useState(FilterKey.All);
+
+  return <RepositoriesFilterComponent value={value} onChange={setValue} />;
+};

extensions/ql-vscode/src/view/variant-analysis/RepositoriesFilter.tsx

@@ -0,0 +1,36 @@
+import * as React from "react";
+import { useCallback } from "react";
+import styled from "styled-components";
+import { VSCodeDropdown, VSCodeOption } from "@vscode/webview-ui-toolkit/react";
+import { Codicon } from "../common";
+import { FilterKey } from "../../pure/variant-analysis-filter-sort";
+
+const Dropdown = styled(VSCodeDropdown)`
+  width: 100%;
+`;
+
+type Props = {
+  value: FilterKey;
+  onChange: (value: FilterKey) => void;
+
+  className?: string;
+};
+
+export const RepositoriesFilter = ({ value, onChange, className }: Props) => {
+  const handleInput = useCallback(
+    (e: InputEvent) => {
+      const target = e.target as HTMLSelectElement;
+
+      onChange(target.value as FilterKey);
+    },
+    [onChange],
+  );
+
+  return (
+    <Dropdown value={value} onInput={handleInput} className={className}>
+      <Codicon name="list-filter" label="Filter..." slot="indicator" />
+      <VSCodeOption value={FilterKey.All}>All</VSCodeOption>
+      <VSCodeOption value={FilterKey.WithResults}>With results</VSCodeOption>
+    </Dropdown>
+  );
+};

extensions/ql-vscode/src/view/variant-analysis/RepositoriesSearchSortRow.tsx

@@ -2,11 +2,13 @@ import * as React from "react";
 import { Dispatch, SetStateAction, useCallback } from "react";
 import styled from "styled-components";
 import {
+  FilterKey,
   RepositoriesFilterSortState,
   SortKey,
 } from "../../pure/variant-analysis-filter-sort";
 import { RepositoriesSearch } from "./RepositoriesSearch";
 import { RepositoriesSort } from "./RepositoriesSort";
+import { RepositoriesFilter } from "./RepositoriesFilter";
 
 type Props = {
   value: RepositoriesFilterSortState;
@@ -25,6 +27,10 @@ const RepositoriesSearchColumn = styled(RepositoriesSearch)`
   flex: 3;
 `;
 
+const RepositoriesFilterColumn = styled(RepositoriesFilter)`
+  flex: 1;
+`;
+
 const RepositoriesSortColumn = styled(RepositoriesSort)`
   flex: 1;
 `;
@@ -40,6 +46,16 @@ export const RepositoriesSearchSortRow = ({ value, onChange }: Props) => {
     [onChange],
   );
 
+  const handleFilterKeyChange = useCallback(
+    (filterKey: FilterKey) => {
+      onChange((oldValue) => ({
+        ...oldValue,
+        filterKey,
+      }));
+    },
+    [onChange],
+  );
+
   const handleSortKeyChange = useCallback(
     (sortKey: SortKey) => {
       onChange((oldValue) => ({
@@ -56,6 +72,10 @@ export const RepositoriesSearchSortRow = ({ value, onChange }: Props) => {
         value={value.searchValue}
         onChange={handleSearchValueChange}
       />
+      <RepositoriesFilterColumn
+        value={value.filterKey}
+        onChange={handleFilterKeyChange}
+      />
       <RepositoriesSortColumn
         value={value.sortKey}
         onChange={handleSortKeyChange}

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisSkippedRepositoriesTab.tsx

@@ -56,8 +56,8 @@ export const VariantAnalysisSkippedRepositoriesTab = ({
 }: VariantAnalysisSkippedRepositoriesTabProps) => {
   const repositories = useMemo(() => {
     return skippedRepositoryGroup.repositories
-      ?.filter((repo) => {
-        return matchesFilter(repo, filterSortState);
+      ?.filter((repository) => {
+        return matchesFilter({ repository }, filterSortState);
       })
       ?.sort(compareRepository(filterSortState));
   }, [filterSortState, skippedRepositoryGroup.repositories]);

extensions/ql-vscode/test/unit-tests/variant-analysis-filter-sort.test.ts

@@ -4,6 +4,7 @@ import {
   defaultFilterSortState,
   filterAndSortRepositoriesWithResults,
   filterAndSortRepositoriesWithResultsByName,
+  FilterKey,
   matchesFilter,
   SortKey,
 } from "../../src/pure/variant-analysis-filter-sort";
@@ -13,32 +14,93 @@ describe(matchesFilter.name, () => {
     fullName: "github/codeql",
   };
 
-  const testCases = [
-    { searchValue: "", matches: true },
-    { searchValue: "github/codeql", matches: true },
-    { searchValue: "github", matches: true },
-    { searchValue: "git", matches: true },
-    { searchValue: "codeql", matches: true },
-    { searchValue: "code", matches: true },
-    { searchValue: "ql", matches: true },
-    { searchValue: "/", matches: true },
-    { searchValue: "gothub/codeql", matches: false },
-    { searchValue: "hello", matches: false },
-    { searchValue: "cod*ql", matches: false },
-    { searchValue: "cod?ql", matches: false },
-  ];
+  describe("searchValue", () => {
+    const testCases = [
+      { searchValue: "", matches: true },
+      { searchValue: "github/codeql", matches: true },
+      { searchValue: "github", matches: true },
+      { searchValue: "git", matches: true },
+      { searchValue: "codeql", matches: true },
+      { searchValue: "code", matches: true },
+      { searchValue: "ql", matches: true },
+      { searchValue: "/", matches: true },
+      { searchValue: "gothub/codeql", matches: false },
+      { searchValue: "hello", matches: false },
+      { searchValue: "cod*ql", matches: false },
+      { searchValue: "cod?ql", matches: false },
+    ];
 
-  test.each(testCases)(
-    "returns $matches if searching for $searchValue",
-    ({ searchValue, matches }) => {
+    test.each(testCases)(
+      "returns $matches if searching for $searchValue",
+      ({ searchValue, matches }) => {
+        expect(
+          matchesFilter(
+            { repository },
+            {
+              ...defaultFilterSortState,
+              searchValue,
+            },
+          ),
+        ).toBe(matches);
+      },
+    );
+  });
+
+  describe("filterKey", () => {
+    it("returns true if filterKey is all and resultCount is positive", () => {
       expect(
-        matchesFilter(repository, {
-          ...defaultFilterSortState,
-          searchValue,
-        }),
-      ).toBe(matches);
-    },
-  );
+        matchesFilter(
+          { repository, resultCount: 1 },
+          { ...defaultFilterSortState, filterKey: FilterKey.All },
+        ),
+      ).toBe(true);
+    });
+
+    it("returns true if filterKey is all and resultCount is zero", () => {
+      expect(
+        matchesFilter(
+          { repository, resultCount: 0 },
+          { ...defaultFilterSortState, filterKey: FilterKey.All },
+        ),
+      ).toBe(true);
+    });
+
+    it("returns true if filterKey is all and resultCount is undefined", () => {
+      expect(
+        matchesFilter(
+          { repository },
+          { ...defaultFilterSortState, filterKey: FilterKey.All },
+        ),
+      ).toBe(true);
+    });
+
+    it("returns true if filterKey is withResults and resultCount is positive", () => {
+      expect(
+        matchesFilter(
+          { repository, resultCount: 1 },
+          { ...defaultFilterSortState, filterKey: FilterKey.WithResults },
+        ),
+      ).toBe(true);
+    });
+
+    it("returns false if filterKey is withResults and resultCount is zero", () => {
+      expect(
+        matchesFilter(
+          { repository, resultCount: 0 },
+          { ...defaultFilterSortState, filterKey: FilterKey.WithResults },
+        ),
+      ).toBe(false);
+    });
+
+    it("returns false if filterKey is withResults and resultCount is undefined", () => {
+      expect(
+        matchesFilter(
+          { repository },
+          { ...defaultFilterSortState, filterKey: FilterKey.WithResults },
+        ),
+      ).toBe(false);
+    });
+  });
 });
 
 describe(compareRepository.name, () => {
@@ -349,7 +411,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
     },
   ];
 
-  describe("when sort key is given without filter", () => {
+  describe("when sort key is given without search or filter", () => {
     it("returns the correct results", () => {
       expect(
         filterAndSortRepositoriesWithResultsByName(repositories, {
@@ -365,7 +427,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
     });
   });
 
-  describe("when sort key and search filter are given", () => {
+  describe("when sort key and search are given without filter", () => {
     it("returns the correct results", () => {
       expect(
         filterAndSortRepositoriesWithResultsByName(repositories, {
@@ -376,6 +438,30 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
       ).toEqual([repositories[2], repositories[0]]);
     });
   });
+
+  describe("when sort key and filter withResults are given without search", () => {
+    it("returns the correct results", () => {
+      expect(
+        filterAndSortRepositoriesWithResultsByName(repositories, {
+          ...defaultFilterSortState,
+          sortKey: SortKey.ResultsCount,
+          filterKey: FilterKey.WithResults,
+        }),
+      ).toEqual([repositories[3], repositories[2], repositories[0]]);
+    });
+  });
+
+  describe("when sort key, search, and filter withResults are given", () => {
+    it("returns the correct results", () => {
+      expect(
+        filterAndSortRepositoriesWithResultsByName(repositories, {
+          sortKey: SortKey.ResultsCount,
+          filterKey: FilterKey.WithResults,
+          searchValue: "r",
+        }),
+      ).toEqual([repositories[3]]);
+    });
+  });
 });
 
 describe(filterAndSortRepositoriesWithResults.name, () => {
@@ -410,7 +496,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
     },
   ];
 
-  describe("when sort key is given without filter", () => {
+  describe("when sort key is given", () => {
     it("returns the correct results", () => {
       expect(
         filterAndSortRepositoriesWithResults(repositories, {
@@ -426,7 +512,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
     });
   });
 
-  describe("when sort key and search filter are given", () => {
+  describe("when sort key and search are given", () => {
     it("returns the correct results", () => {
       expect(
         filterAndSortRepositoriesWithResults(repositories, {
@@ -438,12 +524,49 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
     });
   });
 
-  describe("when sort key, search filter, and repository ids are given", () => {
+  describe("when sort key and filter withResults are given", () => {
     it("returns the correct results", () => {
       expect(
         filterAndSortRepositoriesWithResults(repositories, {
           ...defaultFilterSortState,
           sortKey: SortKey.ResultsCount,
+          filterKey: FilterKey.WithResults,
+        }),
+      ).toEqual([repositories[3], repositories[2], repositories[0]]);
+    });
+  });
+
+  describe("when sort key and filter withResults are given", () => {
+    it("returns the correct results", () => {
+      expect(
+        filterAndSortRepositoriesWithResults(repositories, {
+          ...defaultFilterSortState,
+          sortKey: SortKey.ResultsCount,
+          filterKey: FilterKey.WithResults,
+        }),
+      ).toEqual([repositories[3], repositories[2], repositories[0]]);
+    });
+  });
+
+  describe("when sort key, search, and filter withResults are given", () => {
+    it("returns the correct results", () => {
+      expect(
+        filterAndSortRepositoriesWithResults(repositories, {
+          ...defaultFilterSortState,
+          sortKey: SortKey.ResultsCount,
+          filterKey: FilterKey.WithResults,
+          searchValue: "r",
+        }),
+      ).toEqual([repositories[3]]);
+    });
+  });
+
+  describe("when sort key, search, filter withResults, and repository ids are given", () => {
+    it("returns the correct results", () => {
+      expect(
+        filterAndSortRepositoriesWithResults(repositories, {
+          sortKey: SortKey.ResultsCount,
+          filterKey: FilterKey.WithResults,
           searchValue: "la",
           repositoryIds: [
             repositories[1].repository.id,