Merge pull request #2279 from github/koesie10/fix-result-view-selected

Fix empty result view when switching between queries
github · Apr 11, 2023 · b0940e6 · b0940e6
2 parents 048428b + 24c40af
commit b0940e6
Show file tree

Hide file tree

Showing 2 changed files with 182 additions and 55 deletions.
diff --git a/extensions/ql-vscode/src/view/results/__tests__/results.spec.tsx b/extensions/ql-vscode/src/view/results/__tests__/results.spec.tsx
@@ -1,5 +1,5 @@
 import * as React from "react";
-import { render as reactRender, screen } from "@testing-library/react";
+import { act, render as reactRender, screen } from "@testing-library/react";
 import { ResultsApp } from "../results";
 import {
   Interpretation,
@@ -20,18 +20,20 @@ const exampleSarif = fs.readJSONSync(
 describe(ResultsApp.name, () => {
   const render = () => reactRender(<ResultsApp />);
   const postMessage = async (msg: IntoResultsViewMsg) => {
-    // 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,
-      }),
-    );
+    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));
+      // 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 () => {
@@ -95,6 +97,7 @@ describe(ResultsApp.name, () => {
         },
       },
     };
+
     await postMessage(message);
 
     expect(
@@ -117,4 +120,84 @@ describe(ResultsApp.name, () => {
       screen.getByText("'x' is assigned a value but never used."),
     ).toBeInTheDocument();
   });
+
+  it("renders results when switching between queries with different result set names", async () => {
+    render();
+
+    await postMessage({
+      t: "setState",
+      interpretation: undefined,
+      origResultsPaths: {
+        resultsPath: "/a/b/c/results.bqrs",
+        interpretedResultsPath: "/a/b/c/interpretedResults.sarif",
+      },
+      resultsPath: "/a/b/c/results.bqrs",
+      parsedResultSets: {
+        pageNumber: 0,
+        pageSize: 200,
+        numPages: 1,
+        numInterpretedPages: 0,
+        resultSet: {
+          schema: {
+            name: "#select",
+            rows: 1,
+            columns: [{ kind: "s" }],
+            pagination: { "step-size": 200, offsets: [13] },
+          },
+          rows: [["foobar1"]],
+          t: "RawResultSet",
+        },
+        resultSetNames: ["#select"],
+      },
+      sortedResultsMap: {},
+      database: {
+        name: "test-db",
+        databaseUri: "test-db-uri",
+      },
+      shouldKeepOldResultsWhileRendering: false,
+      metadata: {},
+      queryName: "empty.ql",
+      queryPath: "/a/b/c/empty.ql",
+    });
+
+    expect(screen.getByText("foobar1")).toBeInTheDocument();
+
+    await postMessage({
+      t: "setState",
+      interpretation: undefined,
+      origResultsPaths: {
+        resultsPath: "/a/b/c/results.bqrs",
+        interpretedResultsPath: "/a/b/c/interpretedResults.sarif",
+      },
+      resultsPath: "/a/b/c/results.bqrs",
+      parsedResultSets: {
+        pageNumber: 0,
+        pageSize: 200,
+        numPages: 1,
+        numInterpretedPages: 0,
+        resultSet: {
+          schema: {
+            name: "#Quick_evaluation_of_expression",
+            rows: 1,
+            columns: [{ name: "#expr_result", kind: "s" }],
+            pagination: { "step-size": 200, offsets: [49] },
+          },
+          rows: [["foobar2"]],
+          t: "RawResultSet",
+        },
+        resultSetNames: ["#Quick_evaluation_of_expression"],
+      },
+      sortedResultsMap: {},
+      database: {
+        name: "test-db",
+        databaseUri: "test-db-uri",
+      },
+      shouldKeepOldResultsWhileRendering: false,
+      metadata: {},
+      queryName: "Quick evaluation of empty.ql:1",
+      queryPath: "/a/b/c/empty.ql",
+    });
+
+    expect(screen.getByText("foobar2")).toBeInTheDocument();
+  });
 });
diff --git a/extensions/ql-vscode/src/view/results/result-tables.tsx b/extensions/ql-vscode/src/view/results/result-tables.tsx
@@ -78,6 +78,52 @@ function renderResultCountString(resultSet: ResultSet): JSX.Element {
   );
 }
 
+function getInterpretedTableName(interpretation: Interpretation): string {
+  return interpretation.data.t === "GraphInterpretationData"
+    ? GRAPH_TABLE_NAME
+    : ALERTS_TABLE_NAME;
+}
+
+function getResultSetNames(
+  interpretation: Interpretation | undefined,
+  parsedResultSets: ParsedResultSets,
+): string[] {
+  return interpretation
+    ? parsedResultSets.resultSetNames.concat([
+        getInterpretedTableName(interpretation),
+      ])
+    : parsedResultSets.resultSetNames;
+}
+
+function getResultSets(
+  rawResultSets: readonly ResultSet[],
+  interpretation: Interpretation | undefined,
+): ResultSet[] {
+  const resultSets: ResultSet[] =
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore 2783 Avoid compilation error for overwriting the t property
+    rawResultSets.map((rs) => ({ t: "RawResultSet", ...rs }));
+
+  if (interpretation !== undefined) {
+    const tableName = getInterpretedTableName(interpretation);
+    resultSets.push({
+      t: "InterpretedResultSet",
+      // FIXME: The values of version, columns, tupleCount are
+      // unused stubs because a InterpretedResultSet schema isn't used the
+      // same way as a RawResultSet. Probably should pull `name` field
+      // out.
+      schema: {
+        name: tableName,
+        rows: 1,
+        columns: [],
+      },
+      name: tableName,
+      interpretation,
+    });
+  }
+  return resultSets;
+}
+
 /**
  * Displays multiple `ResultTable` tables, where the table to be displayed is selected by a
  * dropdown.
@@ -86,51 +132,13 @@ export class ResultTables extends React.Component<
   ResultTablesProps,
   ResultTablesState
 > {
-  private getResultSets(): ResultSet[] {
-    const resultSets: ResultSet[] =
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore 2783
-      this.props.rawResultSets.map((rs) => ({ t: "RawResultSet", ...rs }));
-
-    if (this.props.interpretation !== undefined) {
-      const tableName = this.getInterpretedTableName();
-      resultSets.push({
-        t: "InterpretedResultSet",
-        // FIXME: The values of version, columns, tupleCount are
-        // unused stubs because a InterpretedResultSet schema isn't used the
-        // same way as a RawResultSet. Probably should pull `name` field
-        // out.
-        schema: {
-          name: tableName,
-          rows: 1,
-          columns: [],
-        },
-        name: tableName,
-        interpretation: this.props.interpretation,
-      });
-    }
-    return resultSets;
-  }
-
-  private getInterpretedTableName(): string {
-    return this.props.interpretation?.data.t === "GraphInterpretationData"
-      ? GRAPH_TABLE_NAME
-      : ALERTS_TABLE_NAME;
-  }
-
-  private getResultSetNames(): string[] {
-    return this.props.interpretation
-      ? this.props.parsedResultSets.resultSetNames.concat([
-          this.getInterpretedTableName(),
-        ])
-      : this.props.parsedResultSets.resultSetNames;
-  }
-
   constructor(props: ResultTablesProps) {
     super(props);
     const selectedTable =
       props.parsedResultSets.selectedTable ||
-      getDefaultResultSet(this.getResultSets());
+      getDefaultResultSet(
+        getResultSets(props.rawResultSets, props.interpretation),
+      );
     const selectedPage = `${props.parsedResultSets.pageNumber + 1}`;
     this.state = {
       selectedTable,
@@ -139,6 +147,36 @@ export class ResultTables extends React.Component<
     };
   }
 
+  componentDidUpdate(
+    prevProps: Readonly<ResultTablesProps>,
+    prevState: Readonly<ResultTablesState>,
+    snapshot?: any,
+  ) {
+    const resultSetExists =
+      this.props.parsedResultSets.resultSetNames.some(
+        (v) => this.state.selectedTable === v,
+      ) ||
+      getResultSets(this.props.rawResultSets, this.props.interpretation).some(
+        (v) => this.state.selectedTable === v.schema.name,
+      );
+
+    // If the selected result set does not exist, select the default result set.
+    if (!resultSetExists) {
+      this.setState((state, props) => {
+        const selectedTable =
+          props.parsedResultSets.selectedTable ||
+          getDefaultResultSet(
+            getResultSets(props.rawResultSets, props.interpretation),
+          );
+
+        return {
+          selectedTable,
+          selectedPage: `${props.parsedResultSets.pageNumber + 1}`,
+        };
+      });
+    }
+  }
+
   untoggleProblemsView() {
     this.setState({
       problemsViewSelected: false,
@@ -303,8 +341,14 @@ export class ResultTables extends React.Component<
 
   render(): React.ReactNode {
     const { selectedTable } = this.state;
-    const resultSets = this.getResultSets();
-    const resultSetNames = this.getResultSetNames();
+    const resultSets = getResultSets(
+      this.props.rawResultSets,
+      this.props.interpretation,
+    );
+    const resultSetNames = getResultSetNames(
+      this.props.interpretation,
+      this.props.parsedResultSets,
+    );
 
     const resultSet = resultSets.find(
       (resultSet) => resultSet.schema.name === selectedTable,