From 2cf943207c6e5853968ae6e5a2ff687f9ebf2c16 Mon Sep 17 00:00:00 2001
From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Date: Wed, 3 Apr 2024 13:09:53 +0200
Subject: [PATCH] Use MLv2 to determine whether a query can be previewed
(#40609)
* 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
---
.../scenarios/question/notebook.cy.spec.js | 30 +++++++++++++++----
.../notebook/NotebookStep/NotebookStep.tsx | 3 +-
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/e2e/test/scenarios/question/notebook.cy.spec.js b/e2e/test/scenarios/question/notebook.cy.spec.js
index b33bfcecbe4..431d1d5aee8 100644
--- a/e2e/test/scenarios/question/notebook.cy.spec.js
+++ b/e2e/test/scenarios/question/notebook.cy.spec.js
@@ -678,9 +678,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");
@@ -690,13 +708,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").blur();
+ getNotebookStep("limit").within(() => {
+ cy.findByPlaceholderText("Enter a limit").type("5").realPress("Tab");
cy.icon("play").click();
assertTableRowCount(5);
- cy.findByDisplayValue("5").type("{selectall}50").blur();
+ cy.findByDisplayValue("5").type("{selectall}50").realPress("Tab");
cy.button("Refresh").click();
assertTableRowCount(10);
});
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.tsx b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.tsx
index 0ce555acac9..b1519c9259e 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.tsx
@@ -9,6 +9,7 @@ import { useToggle } from "metabase/hooks/use-toggle";
import { color as c } from "metabase/lib/colors";
import { Icon } from "metabase/ui";
import type { Query } from "metabase-lib";
+import * as Lib from "metabase-lib";
import type Question from "metabase-lib/v1/Question";
import NotebookStepPreview from "../NotebookStepPreview";
@@ -105,7 +106,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;
--
GitLab