Skip to content

Commit

Permalink
Use MLv2 to determine whether a query can be previewed (#40609) (#40946)
Browse files Browse the repository at this point in the history
* Use MLv2 to determine whether a query can be previewed

* Add E2E repro for #40608

* Expand E2E repro

* Fix test

* Make sure the step is active and visible before offering to preview its query

* Expand E2E test

* Address review comment - use `getNotebookStep` helper

Co-authored-by: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
  • Loading branch information
metabase-bot[bot] and nemanjaglumac committed Apr 11, 2024
1 parent 7453405 commit 7f10cc6
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
30 changes: 24 additions & 6 deletions e2e/test/scenarios/question/notebook.cy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,9 +641,27 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => {
});
});

it("should properly render previews (metabase#28726), (metabase#29959)", () => {
openOrdersTable({ mode: "notebook" });
cy.findByTestId("step-data-0-0").within(() => {
it("should properly render previews (metabase#28726, metabase#29959, metabase#40608)", () => {
startNewQuestion();

cy.log(
"Preview should not be possible without the source data (metabase#40608)",
);
getNotebookStep("data")
.as("dataStep")
.within(() => {
cy.findByText("Pick your starting data").should("exist");
cy.icon("play").should("not.be.visible");
});

popover().findByTextEnsureVisible("Raw Data").click();
cy.get("@dataStep").icon("play").should("not.be.visible");
popover().findByTextEnsureVisible("Orders").click();

getNotebookStep("filter").icon("play").should("not.be.visible");
getNotebookStep("summarize").icon("play").should("not.be.visible");

cy.get("@dataStep").within(() => {
cy.icon("play").click();
assertTableRowCount(10);
cy.findByTextEnsureVisible("Subtotal");
Expand All @@ -653,13 +671,13 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => {
});

cy.button("Row limit").click();
cy.findByTestId("step-limit-0-0").within(() => {
cy.findByPlaceholderText("Enter a limit").type("5");
getNotebookStep("limit").within(() => {
cy.findByPlaceholderText("Enter a limit").type("5").realPress("Tab");

cy.icon("play").click();
assertTableRowCount(5);

cy.findByDisplayValue("5").type("{selectall}50");
cy.findByDisplayValue("5").type("{selectall}50").realPress("Tab");
cy.button("Refresh").click();
assertTableRowCount(10);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import IconButtonWrapper from "metabase/components/IconButtonWrapper";
import { useToggle } from "metabase/hooks/use-toggle";
import { color as c } from "metabase/lib/colors";
import { Icon } from "metabase/ui";
import * as Lib from "metabase-lib";
import type Question from "metabase-lib/Question";
import type { Query } from "metabase-lib/types";

Expand Down Expand Up @@ -104,7 +105,7 @@ function NotebookStep({
} = STEP_UI[step.type] || {};

const color = getColor();
const canPreview = Boolean(step.getPreviewQuery);
const canPreview = Lib.canRun(step.query) && step.active && step.visible;
const hasPreviewButton = !isPreviewOpen && canPreview;
const canRevert = typeof step.revert === "function" && !readOnly;

Expand Down

0 comments on commit 7f10cc6

Please sign in to comment.