Add config options for default sorting/filtering values in variant analysis results view (#2392)

This commit is contained in:
Shati Patel
2023-05-03 15:52:35 +01:00
committed by GitHub
parent 70a9ee6b1d
commit 71611e03fe
14 changed files with 192 additions and 69 deletions

View File

@@ -2,6 +2,8 @@
## [UNRELEASED]
- Add settings `codeQL.variantAnalysis.defaultResultsFilter` and `codeQL.variantAnalysis.defaultResultsSort` for configuring how variant analysis results are filtered and sorted in the results view. The default is to show all repositories, and to sort by the number of results. [#2392](https://github.com/github/vscode-codeql/pull/2392)
## 1.8.4 - 3 May 2023
- Avoid repeated error messages when unable to monitor a variant analysis. [#2396](https://github.com/github/vscode-codeql/pull/2396)

View File

@@ -329,6 +329,36 @@
"patternErrorMessage": "Please enter a valid GitHub repository",
"markdownDescription": "[For internal use only] The name of the GitHub repository in which the GitHub Actions workflow is run when using the \"Run Variant Analysis\" command. The repository should be of the form `<owner>/<repo>`)."
},
"codeQL.variantAnalysis.defaultResultsFilter": {
"type": "string",
"default": "all",
"enum": [
"all",
"withResults"
],
"enumDescriptions": [
"Show all repositories in the results view.",
"Show only repositories with results in the results view."
],
"description": "The default filter to apply to the variant analysis results view."
},
"codeQL.variantAnalysis.defaultResultsSort": {
"type": "string",
"default": "numberOfResults",
"enum": [
"alphabetically",
"popularity",
"mostRecentCommit",
"numberOfResults"
],
"enumDescriptions": [
"Sort repositories alphabetically in the results view.",
"Sort repositories by popularity in the results view.",
"Sort repositories by most recent commit in the results view.",
"Sort repositories by number of results in the results view."
],
"description": "The default sorting order for repositories in the variant analysis results view."
},
"codeQL.logInsights.joinOrderWarningThreshold": {
"type": "number",
"default": 50,

View File

@@ -9,6 +9,11 @@ import {
import { DistributionManager } from "./codeql-cli/distribution";
import { extLogger } from "./common";
import { ONE_DAY_IN_MS } from "./pure/time";
import {
FilterKey,
SortKey,
defaultFilterSortState,
} from "./pure/variant-analysis-filter-sort";
export const ALL_SETTINGS: Setting[] = [];
@@ -541,6 +546,34 @@ export class VariantAnalysisConfigListener
}
}
const VARIANT_ANALYSIS_FILTER_RESULTS = new Setting(
"defaultResultsFilter",
VARIANT_ANALYSIS_SETTING,
);
export function getVariantAnalysisDefaultResultsFilter(): FilterKey {
const value = VARIANT_ANALYSIS_FILTER_RESULTS.getValue<string>();
if (Object.values(FilterKey).includes(value as FilterKey)) {
return value as FilterKey;
} else {
return defaultFilterSortState.filterKey;
}
}
const VARIANT_ANALYSIS_SORT_RESULTS = new Setting(
"defaultResultsSort",
VARIANT_ANALYSIS_SETTING,
);
export function getVariantAnalysisDefaultResultsSort(): SortKey {
const value = VARIANT_ANALYSIS_SORT_RESULTS.getValue<string>();
if (Object.values(SortKey).includes(value as SortKey)) {
return value as SortKey;
} else {
return defaultFilterSortState.sortKey;
}
}
/**
* The branch of "github/codeql-variant-analysis-action" to use with the "Run Variant Analysis" command.
* Default value is "main".

View File

@@ -11,7 +11,10 @@ import {
VariantAnalysisScannedRepositoryResult,
VariantAnalysisScannedRepositoryState,
} from "../variant-analysis/shared/variant-analysis";
import { RepositoriesFilterSortStateWithIds } from "./variant-analysis-filter-sort";
import {
RepositoriesFilterSortState,
RepositoriesFilterSortStateWithIds,
} from "./variant-analysis-filter-sort";
import { ErrorLike } from "./errors";
import { DataFlowPaths } from "../variant-analysis/shared/data-flow-paths";
import { ExternalApiUsage } from "../data-extensions-editor/external-api-usage";
@@ -407,6 +410,11 @@ export interface SetVariantAnalysisMessage {
variantAnalysis: VariantAnalysis;
}
export interface SetFilterSortStateMessage {
t: "setFilterSortState";
filterSortState: RepositoriesFilterSortState;
}
export type VariantAnalysisState = {
variantAnalysisId: number;
};
@@ -459,6 +467,7 @@ export interface ShowDataFlowPathsMessage {
export type ToVariantAnalysisMessage =
| SetVariantAnalysisMessage
| SetFilterSortStateMessage
| SetRepoResultsMessage
| SetRepoStatesMessage;

View File

@@ -30,7 +30,7 @@ export type RepositoriesFilterSortStateWithIds = RepositoriesFilterSortState & {
export const defaultFilterSortState: RepositoriesFilterSortState = {
searchValue: "",
filterKey: FilterKey.All,
sortKey: SortKey.Alphabetically,
sortKey: SortKey.NumberOfResults,
};
export function matchesFilter(

View File

@@ -27,6 +27,10 @@ import { redactableError } from "../pure/errors";
import { DataFlowPathsView } from "./data-flow-paths-view";
import { DataFlowPaths } from "./shared/data-flow-paths";
import { App } from "../common/app";
import {
getVariantAnalysisDefaultResultsFilter,
getVariantAnalysisDefaultResultsSort,
} from "../config";
export class VariantAnalysisView
extends AbstractWebview<ToVariantAnalysisMessage, FromVariantAnalysisMessage>
@@ -186,11 +190,22 @@ export class VariantAnalysisView
return;
}
const filterSortState = {
searchValue: "",
filterKey: getVariantAnalysisDefaultResultsFilter(),
sortKey: getVariantAnalysisDefaultResultsSort(),
};
await this.postMessage({
t: "setVariantAnalysis",
variantAnalysis,
});
await this.postMessage({
t: "setFilterSortState",
filterSortState,
});
const repoStates = await this.manager.getRepoStates(this.variantAnalysisId);
if (repoStates.length === 0) {
return;

View File

@@ -0,0 +1,19 @@
import { act } from "@testing-library/react";
/** Helper function used in tests */
export async function postMessage<T>(msg: T): Promise<void> {
await act(async () => {
// window.postMessage doesn't set the origin correctly, see
// https://github.com/jsdom/jsdom/issues/2745
window.dispatchEvent(
new MessageEvent("message", {
source: window,
origin: window.location.origin,
data: msg,
}),
);
// The event is dispatched asynchronously, so we need to wait for it to be handled.
await new Promise((resolve) => setTimeout(resolve, 0));
});
}

View File

@@ -1,5 +1,5 @@
import * as React from "react";
import { act, render as reactRender, screen } from "@testing-library/react";
import { render as reactRender, screen } from "@testing-library/react";
import { ResultsApp } from "../results";
import {
Interpretation,
@@ -9,6 +9,7 @@ import {
import * as fs from "fs-extra";
import { resolve } from "path";
import { ColumnKindCode } from "../../../pure/bqrs-cli-types";
import { postMessage } from "../../common/post-message";
const exampleSarif = fs.readJSONSync(
resolve(
@@ -19,22 +20,6 @@ const exampleSarif = fs.readJSONSync(
describe(ResultsApp.name, () => {
const render = () => reactRender(<ResultsApp />);
const postMessage = async (msg: IntoResultsViewMsg) => {
await act(async () => {
// window.postMessage doesn't set the origin correctly, see
// https://github.com/jsdom/jsdom/issues/2745
window.dispatchEvent(
new MessageEvent("message", {
source: window,
origin: window.location.origin,
data: msg,
}),
);
// The event is dispatched asynchronously, so we need to wait for it to be handled.
await new Promise((resolve) => setTimeout(resolve, 0));
});
};
it("renders results", async () => {
render();
@@ -98,13 +83,13 @@ describe(ResultsApp.name, () => {
},
};
await postMessage(message);
await postMessage<IntoResultsViewMsg>(message);
expect(
screen.getByText("'x' is assigned a value but never used."),
).toBeInTheDocument();
await postMessage({
await postMessage<IntoResultsViewMsg>({
...message,
t: "showInterpretedPage",
pageNumber: 1,
@@ -124,7 +109,7 @@ describe(ResultsApp.name, () => {
it("renders results when switching between queries with different result set names", async () => {
render();
await postMessage({
await postMessage<IntoResultsViewMsg>({
t: "setState",
interpretation: undefined,
origResultsPaths: {
@@ -162,7 +147,7 @@ describe(ResultsApp.name, () => {
expect(screen.getByText("foobar1")).toBeInTheDocument();
await postMessage({
await postMessage<IntoResultsViewMsg>({
t: "setState",
interpretation: undefined,
origResultsPaths: {

View File

@@ -87,6 +87,8 @@ export function VariantAnalysis({
vscode.setState({
variantAnalysisId: msg.variantAnalysis.id,
});
} else if (msg.t === "setFilterSortState") {
setFilterSortState(msg.filterSortState);
} else if (msg.t === "setRepoResults") {
setRepoResults((oldRepoResults) => {
const newRepoIds = msg.repoResults.map((r) => r.repositoryId);

View File

@@ -1,11 +1,14 @@
import * as React from "react";
import { render as reactRender, screen } from "@testing-library/react";
import { render as reactRender, screen, waitFor } from "@testing-library/react";
import {
VariantAnalysisFailureReason,
VariantAnalysisStatus,
} from "../../../variant-analysis/shared/variant-analysis";
import { VariantAnalysis, VariantAnalysisProps } from "../VariantAnalysis";
import { createMockVariantAnalysis } from "../../../../test/factories/variant-analysis/shared/variant-analysis";
import { ToVariantAnalysisMessage } from "../../../pure/interface-types";
import { FilterKey, SortKey } from "../../../pure/variant-analysis-filter-sort";
import { postMessage } from "../../common/post-message";
describe(VariantAnalysis.name, () => {
const render = (props: Partial<VariantAnalysisProps> = {}) =>
@@ -46,4 +49,29 @@ describe(VariantAnalysis.name, () => {
),
).toBeInTheDocument();
});
it("renders results view with correct filter and sort state", async () => {
const variantAnalysis = createMockVariantAnalysis({});
render({ variantAnalysis });
await waitFor(() => screen.getByDisplayValue("All"));
await waitFor(() => screen.getByDisplayValue("Number of results"));
await postMessage<ToVariantAnalysisMessage>({
t: "setFilterSortState",
filterSortState: {
searchValue: "",
filterKey: FilterKey.WithResults,
sortKey: SortKey.Alphabetically,
},
});
expect(screen.getByDisplayValue("With results")).toBeInTheDocument();
expect(screen.getByDisplayValue("Alphabetically")).toBeInTheDocument();
expect(screen.queryByDisplayValue("All")).not.toBeInTheDocument();
expect(
screen.queryByDisplayValue("Number of results"),
).not.toBeInTheDocument();
});
});

View File

@@ -13,10 +13,8 @@ import {
import { createMockVariantAnalysis } from "../../../../test/factories/variant-analysis/shared/variant-analysis";
import { createMockRepositoryWithMetadata } from "../../../../test/factories/variant-analysis/shared/repository";
import { createMockScannedRepo } from "../../../../test/factories/variant-analysis/shared/scanned-repositories";
import {
defaultFilterSortState,
SortKey,
} from "../../../pure/variant-analysis-filter-sort";
import { SortKey } from "../../../pure/variant-analysis-filter-sort";
import { permissiveFilterSortState } from "../../../../test/unit-tests/variant-analysis-filter-sort.test";
describe(VariantAnalysisAnalyzedRepos.name, () => {
const defaultVariantAnalysis = createMockVariantAnalysis({
@@ -170,7 +168,7 @@ describe(VariantAnalysisAnalyzedRepos.name, () => {
it("uses the search value", () => {
render({
filterSortState: {
...defaultFilterSortState,
...permissiveFilterSortState,
searchValue: "world-2",
},
});
@@ -190,7 +188,7 @@ describe(VariantAnalysisAnalyzedRepos.name, () => {
it("uses the sort key", async () => {
render({
filterSortState: {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.Popularity,
},
});
@@ -209,7 +207,7 @@ describe(VariantAnalysisAnalyzedRepos.name, () => {
it("uses the 'Number of results' sort key", async () => {
render({
filterSortState: {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
},
});

View File

@@ -4,10 +4,8 @@ import {
VariantAnalysisSkippedRepositoriesTab,
VariantAnalysisSkippedRepositoriesTabProps,
} from "../VariantAnalysisSkippedRepositoriesTab";
import {
defaultFilterSortState,
SortKey,
} from "../../../pure/variant-analysis-filter-sort";
import { SortKey } from "../../../pure/variant-analysis-filter-sort";
import { permissiveFilterSortState } from "../../../../test/unit-tests/variant-analysis-filter-sort.test";
describe(VariantAnalysisSkippedRepositoriesTab.name, () => {
const render = (props: VariantAnalysisSkippedRepositoriesTabProps) =>
@@ -142,7 +140,7 @@ describe(VariantAnalysisSkippedRepositoriesTab.name, () => {
],
},
filterSortState: {
...defaultFilterSortState,
...permissiveFilterSortState,
searchValue: "world",
},
});
@@ -177,7 +175,7 @@ describe(VariantAnalysisSkippedRepositoriesTab.name, () => {
],
},
filterSortState: {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.Popularity,
},
});
@@ -210,7 +208,7 @@ describe(VariantAnalysisSkippedRepositoriesTab.name, () => {
],
},
filterSortState: {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
},
});

View File

@@ -1,7 +1,6 @@
import {
compareRepository,
compareWithResults,
defaultFilterSortState,
filterAndSortRepositoriesWithResults,
filterAndSortRepositoriesWithResultsByName,
FilterKey,
@@ -9,6 +8,13 @@ import {
SortKey,
} from "../../src/pure/variant-analysis-filter-sort";
/** A filterSortState that matches everything */
export const permissiveFilterSortState = {
searchValue: "",
filterKey: FilterKey.All,
sortKey: SortKey.Alphabetically,
};
describe(matchesFilter.name, () => {
const repository = {
fullName: "github/codeql",
@@ -37,7 +43,7 @@ describe(matchesFilter.name, () => {
matchesFilter(
{ repository },
{
...defaultFilterSortState,
...permissiveFilterSortState,
searchValue,
},
),
@@ -51,7 +57,7 @@ describe(matchesFilter.name, () => {
expect(
matchesFilter(
{ repository, resultCount: 1 },
{ ...defaultFilterSortState, filterKey: FilterKey.All },
{ ...permissiveFilterSortState, filterKey: FilterKey.All },
),
).toBe(true);
});
@@ -60,7 +66,7 @@ describe(matchesFilter.name, () => {
expect(
matchesFilter(
{ repository, resultCount: 0 },
{ ...defaultFilterSortState, filterKey: FilterKey.All },
{ ...permissiveFilterSortState, filterKey: FilterKey.All },
),
).toBe(true);
});
@@ -69,7 +75,7 @@ describe(matchesFilter.name, () => {
expect(
matchesFilter(
{ repository },
{ ...defaultFilterSortState, filterKey: FilterKey.All },
{ ...permissiveFilterSortState, filterKey: FilterKey.All },
),
).toBe(true);
});
@@ -78,7 +84,7 @@ describe(matchesFilter.name, () => {
expect(
matchesFilter(
{ repository, resultCount: 1 },
{ ...defaultFilterSortState, filterKey: FilterKey.WithResults },
{ ...permissiveFilterSortState, filterKey: FilterKey.WithResults },
),
).toBe(true);
});
@@ -87,7 +93,7 @@ describe(matchesFilter.name, () => {
expect(
matchesFilter(
{ repository, resultCount: 0 },
{ ...defaultFilterSortState, filterKey: FilterKey.WithResults },
{ ...permissiveFilterSortState, filterKey: FilterKey.WithResults },
),
).toBe(false);
});
@@ -96,7 +102,7 @@ describe(matchesFilter.name, () => {
expect(
matchesFilter(
{ repository },
{ ...defaultFilterSortState, filterKey: FilterKey.WithResults },
{ ...permissiveFilterSortState, filterKey: FilterKey.WithResults },
),
).toBe(false);
});
@@ -129,7 +135,7 @@ describe(compareRepository.name, () => {
describe("when sort key is 'Alphabetically'", () => {
const sorter = compareRepository({
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.Alphabetically,
});
@@ -155,7 +161,7 @@ describe(compareRepository.name, () => {
describe("when sort key is 'Popularity'", () => {
const sorter = compareRepository({
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.Popularity,
});
@@ -201,7 +207,7 @@ describe(compareRepository.name, () => {
describe("when sort key is 'Most recent commit'", () => {
const sorter = compareRepository({
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.MostRecentCommit,
});
@@ -273,7 +279,7 @@ describe(compareWithResults.name, () => {
describe("when sort key is 'Popularity'", () => {
const sorter = compareWithResults({
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.Popularity,
});
@@ -299,7 +305,7 @@ describe(compareWithResults.name, () => {
describe("when sort key is 'Most recent commit'", () => {
const sorter = compareWithResults({
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.MostRecentCommit,
});
@@ -325,7 +331,7 @@ describe(compareWithResults.name, () => {
describe("when sort key is results count", () => {
const sorter = compareWithResults({
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
});
@@ -415,7 +421,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResultsByName(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
}),
).toEqual([
@@ -431,7 +437,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResultsByName(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
searchValue: "la",
}),
@@ -443,7 +449,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResultsByName(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
filterKey: FilterKey.WithResults,
}),
@@ -500,7 +506,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
}),
).toEqual([
@@ -516,7 +522,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
searchValue: "la",
}),
@@ -528,7 +534,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
filterKey: FilterKey.WithResults,
}),
@@ -540,7 +546,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
filterKey: FilterKey.WithResults,
}),
@@ -552,7 +558,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
filterKey: FilterKey.WithResults,
searchValue: "r",
@@ -581,7 +587,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
repositoryIds: [
repositories[0].repository.id,
repositories[3].repository.id,
@@ -595,7 +601,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
...permissiveFilterSortState,
repositoryIds: [],
}),
).toEqual([

View File

@@ -36,10 +36,7 @@ import {
import { createTimestampFile } from "../../../../src/helpers";
import { createMockVariantAnalysisRepoTask } from "../../../factories/variant-analysis/gh-api/variant-analysis-repo-task";
import { VariantAnalysisRepoTask } from "../../../../src/variant-analysis/gh-api/variant-analysis";
import {
defaultFilterSortState,
SortKey,
} from "../../../../src/pure/variant-analysis-filter-sort";
import { SortKey } from "../../../../src/pure/variant-analysis-filter-sort";
import { DbManager } from "../../../../src/databases/db-manager";
import { App } from "../../../../src/common/app";
import { ExtensionApp } from "../../../../src/common/vscode/vscode-app";
@@ -49,6 +46,7 @@ import {
REPO_STATES_FILENAME,
writeRepoStates,
} from "../../../../src/variant-analysis/repo-states-store";
import { permissiveFilterSortState } from "../../../unit-tests/variant-analysis-filter-sort.test";
// up to 3 minutes per test
jest.setTimeout(3 * 60 * 1000);
@@ -738,9 +736,9 @@ describe("Variant Analysis Manager", () => {
expect(parsed).toEqual({
name: "new-repo-list",
repositories: [
scannedRepos[4].repository.fullName,
scannedRepos[2].repository.fullName,
scannedRepos[0].repository.fullName,
scannedRepos[4].repository.fullName,
],
});
});
@@ -749,7 +747,7 @@ describe("Variant Analysis Manager", () => {
await variantAnalysisManager.copyRepoListToClipboard(
variantAnalysis.id,
{
...defaultFilterSortState,
...permissiveFilterSortState,
sortKey: SortKey.NumberOfResults,
},
);
@@ -772,7 +770,7 @@ describe("Variant Analysis Manager", () => {
await variantAnalysisManager.copyRepoListToClipboard(
variantAnalysis.id,
{
...defaultFilterSortState,
...permissiveFilterSortState,
searchValue: "ban",
},
);