diff --git a/e2e/support/helpers/e2e-collection-helpers.js b/e2e/support/helpers/e2e-collection-helpers.js index 9c040e9dd5960..28e0f418682cc 100644 --- a/e2e/support/helpers/e2e-collection-helpers.js +++ b/e2e/support/helpers/e2e-collection-helpers.js @@ -1,4 +1,10 @@ -import { entityPickerModal, getFullName, popover } from "e2e/support/helpers"; +import { + entityPickerModal, + entityPickerModalLevel, + entityPickerModalTab, + getFullName, + popover, +} from "e2e/support/helpers"; /** * Clicks the "+" icon on the collection page and selects one of the menu options @@ -86,12 +92,12 @@ export const moveOpenedCollectionTo = newParent => { export function pickEntity({ path, select, tab }) { if (tab) { - cy.findByRole("tab", { name: tab }).click(); + entityPickerModalTab(tab).click(); } if (path) { cy.findByTestId("nested-item-picker").within(() => { for (const [index, name] of path.entries()) { - cy.findByTestId(`item-picker-level-${index}`).findByText(name).click(); + entityPickerModalLevel(index).findByText(name).click(); } }); } diff --git a/e2e/support/helpers/e2e-notebook-helpers.ts b/e2e/support/helpers/e2e-notebook-helpers.ts index b2be12a211a39..d23fc75102527 100644 --- a/e2e/support/helpers/e2e-notebook-helpers.ts +++ b/e2e/support/helpers/e2e-notebook-helpers.ts @@ -1,6 +1,10 @@ import type { CyHttpMessages } from "cypress/types/net-stubbing"; -import { popover } from "e2e/support/helpers/e2e-ui-elements-helpers"; +import { + entityPickerModal, + entityPickerModalTab, + popover, +} from "e2e/support/helpers/e2e-ui-elements-helpers"; import type { NotebookStepType } from "metabase/query_builder/components/notebook/types"; /** @@ -143,7 +147,11 @@ export function joinTable( lhsColumnName?: string, rhsColumnName?: string, ) { - popover().findByText(tableName).click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText(tableName).click(); + }); + if (lhsColumnName && rhsColumnName) { popover().findByText(lhsColumnName).click(); popover().findByText(rhsColumnName).click(); @@ -166,25 +174,22 @@ export function selectSavedQuestionsToJoin( secondQuestionName: string, ) { cy.intercept("GET", "/api/database/*/schemas").as("loadSchemas"); - cy.findAllByTestId("data-bucket-list-item") - .contains("Saved Questions") - .click(); + entityPickerModal().within(() => { + entityPickerModalTab("Models").should("exist"); + entityPickerModalTab("Tables").should("exist"); + entityPickerModalTab("Saved questions").click(); + cy.findByText(firstQuestionName).click(); + }); - cy.findByTestId("select-list") - .findAllByRole("menuitem") - .contains(firstQuestionName) - .click(); cy.wait("@loadSchemas"); // join to question b cy.icon("join_left_outer").click(); - popover().within(() => { - cy.findByText("Sample Database").should("be.visible").click(); - cy.findByText("Raw Data").should("be.visible").click(); - cy.findAllByTestId("data-bucket-list-item") - .contains("Saved Questions") - .click(); + entityPickerModal().within(() => { + entityPickerModalTab("Models").should("exist"); + entityPickerModalTab("Tables").should("exist"); + entityPickerModalTab("Saved questions").click(); cy.findByText(secondQuestionName).click(); }); } diff --git a/e2e/support/helpers/e2e-ui-elements-helpers.js b/e2e/support/helpers/e2e-ui-elements-helpers.js index 4517f07e7d0c2..4220f83903628 100644 --- a/e2e/support/helpers/e2e-ui-elements-helpers.js +++ b/e2e/support/helpers/e2e-ui-elements-helpers.js @@ -32,6 +32,18 @@ export function entityPickerModal() { return cy.findByTestId("entity-picker-modal"); } +export function entityPickerModalLevel(level) { + return cy.findByTestId(`item-picker-level-${level}`); +} + +export function entityPickerModalItem(level, name) { + return entityPickerModalLevel(level).findByText(name).parents("button"); +} + +export function entityPickerModalTab(name) { + return cy.findAllByRole("tab").filter(`:contains(${name})`); +} + export function collectionOnTheGoModal() { return cy.findByTestId("create-collection-on-the-go"); } diff --git a/e2e/test/scenarios/admin/datamodel/editor.cy.spec.js b/e2e/test/scenarios/admin/datamodel/editor.cy.spec.js index 1d4e5b5221ea5..06db2828de370 100644 --- a/e2e/test/scenarios/admin/datamodel/editor.cy.spec.js +++ b/e2e/test/scenarios/admin/datamodel/editor.cy.spec.js @@ -6,15 +6,17 @@ import { import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { describeEE, + entityPickerModal, + entityPickerModalTab, + moveDnDKitElement, openOrdersTable, openProductsTable, openReviewsTable, + openTable, popover, restore, - startNewQuestion, setTokenFeatures, - openTable, - moveDnDKitElement, + startNewQuestion, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PRODUCTS_ID, REVIEWS, REVIEWS_ID, PEOPLE_ID } = @@ -54,8 +56,8 @@ describe("scenarios > admin > datamodel > editor", () => { cy.findByText("Updated Table display_name").should("be.visible"); startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("People").should("be.visible"); cy.findByText("New orders").should("be.visible"); }); @@ -101,8 +103,8 @@ describe("scenarios > admin > datamodel > editor", () => { cy.findByText("5 Hidden Tables").should("be.visible"); startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("People").should("be.visible"); cy.findByText("Orders").should("not.exist"); }); @@ -115,8 +117,8 @@ describe("scenarios > admin > datamodel > editor", () => { cy.findByText("4 Hidden Tables").should("be.visible"); startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("People").should("be.visible"); cy.findByText("Orders").should("be.visible"); }); @@ -227,7 +229,8 @@ describe("scenarios > admin > datamodel > editor", () => { openTable({ database: SAMPLE_DB_ID, table: ORDERS_ID, mode: "notebook" }); cy.icon("join_left_outer").click(); - popover().within(() => { + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Products").click(); }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage @@ -409,7 +412,8 @@ describe("scenarios > admin > datamodel > editor", () => { openTable({ database: SAMPLE_DB_ID, table: ORDERS_ID, mode: "notebook" }); cy.icon("join_left_outer").click(); - popover().within(() => { + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Products").click(); }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage @@ -438,8 +442,8 @@ describe("scenarios > admin > datamodel > editor", () => { cy.signInAsNormalUser(); startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("People").should("be.visible"); cy.findByText("New orders").should("be.visible"); }); @@ -507,7 +511,8 @@ describe("scenarios > admin > datamodel > editor", () => { cy.signInAsNormalUser(); openTable({ database: SAMPLE_DB_ID, table: ORDERS_ID, mode: "notebook" }); cy.icon("join_left_outer").click(); - popover().within(() => { + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Products").click(); }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage diff --git a/e2e/test/scenarios/admin/datamodel/hide_tables.cy.spec.js b/e2e/test/scenarios/admin/datamodel/hide_tables.cy.spec.js index bcebeb0edafd9..ad370abe747d8 100644 --- a/e2e/test/scenarios/admin/datamodel/hide_tables.cy.spec.js +++ b/e2e/test/scenarios/admin/datamodel/hide_tables.cy.spec.js @@ -1,6 +1,11 @@ import { SAMPLE_DB_ID, SAMPLE_DB_SCHEMA_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; -import { restore, startNewQuestion } from "e2e/support/helpers"; +import { + entityPickerModal, + entityPickerModalTab, + restore, + startNewQuestion, +} from "e2e/support/helpers"; const { ORDERS_ID } = SAMPLE_DATABASE; @@ -31,12 +36,11 @@ describe("scenarios > admin > datamodel > hidden tables (metabase#9759)", () => // It shouldn't show in a new question data picker startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Raw Data").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Products"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Orders").should("not.exist"); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.contains("Products").should("exist"); + cy.contains("Orders").should("not.exist"); + }); }); }); diff --git a/e2e/test/scenarios/binning/binning-reproductions.cy.spec.js b/e2e/test/scenarios/binning/binning-reproductions.cy.spec.js index 5534b606e7579..ec5c3d49c9af3 100644 --- a/e2e/test/scenarios/binning/binning-reproductions.cy.spec.js +++ b/e2e/test/scenarios/binning/binning-reproductions.cy.spec.js @@ -1,6 +1,7 @@ import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { + entityPickerModal, restore, popover, visualize, @@ -14,6 +15,7 @@ import { rightSidebar, chartPathWithFillColor, cartesianChartCircle, + entityPickerModalTab, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE; @@ -31,10 +33,10 @@ describe("binning related reproductions", () => { }); startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("16327").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("16327").click(); + }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Pick the metric you want to see").click(); @@ -103,8 +105,8 @@ describe("binning related reproductions", () => { ); startNewQuestion(); - popover().within(() => { - cy.findByText("Saved Questions").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); cy.findByText("17975").click(); }); @@ -143,10 +145,8 @@ describe("binning related reproductions", () => { cy.icon("join_left_outer").click(); - popover().within(() => { - cy.findByTextEnsureVisible("Sample Database").click(); - cy.findByTextEnsureVisible("Raw Data").click(); - cy.findByTextEnsureVisible("Saved Questions").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); cy.findByText("18646").click(); }); @@ -191,8 +191,8 @@ describe("binning related reproductions", () => { // it is essential for this repro to find question following these exact steps // (for example, visiting `/collection/root` would yield different result) startNewQuestion(); - popover().within(() => { - cy.findByText("Saved Questions").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); cy.findByText("11439").click(); }); @@ -380,8 +380,10 @@ describe("binning related reproductions", () => { function openSummarizeOptions(questionType) { startNewQuestion(); - cy.findByText("Saved Questions").click(); - cy.findByText("16379").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("16379").click(); + }); if (questionType === "Simple mode") { visualize(); diff --git a/e2e/test/scenarios/binning/qb-explicit-joins.cy.spec.js b/e2e/test/scenarios/binning/qb-explicit-joins.cy.spec.js index 6f0055f4b4786..7dce4ea42b3b3 100644 --- a/e2e/test/scenarios/binning/qb-explicit-joins.cy.spec.js +++ b/e2e/test/scenarios/binning/qb-explicit-joins.cy.spec.js @@ -1,5 +1,6 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { + entityPickerModal, restore, visualize, changeBinningForDimension, @@ -8,6 +9,7 @@ import { echartsContainer, cartesianChartCircle, chartPathWithFillColor, + entityPickerModalTab, } from "e2e/support/helpers"; const { ORDERS_ID, ORDERS, PEOPLE_ID, PEOPLE, PRODUCTS_ID, PRODUCTS } = @@ -65,10 +67,11 @@ describe("scenarios > binning > from a saved QB question with explicit joins", ( context("via simple mode", () => { beforeEach(() => { startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("QB Binning").click(); + + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("QB Binning").click(); + }); visualize(); summarize(); @@ -130,10 +133,11 @@ describe("scenarios > binning > from a saved QB question with explicit joins", ( context("via notebook mode", () => { beforeEach(() => { startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("QB Binning").click(); + + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("QB Binning").click(); + }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Pick the metric you want to see").click(); @@ -202,10 +206,12 @@ describe("scenarios > binning > from a saved QB question with explicit joins", ( context("via column popover", () => { beforeEach(() => { startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("QB Binning").click(); + + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("QB Binning").click(); + }); + visualize(); }); diff --git a/e2e/test/scenarios/collections/collections.cy.spec.js b/e2e/test/scenarios/collections/collections.cy.spec.js index a4925cc77e437..2c37f62dffe90 100644 --- a/e2e/test/scenarios/collections/collections.cy.spec.js +++ b/e2e/test/scenarios/collections/collections.cy.spec.js @@ -72,7 +72,7 @@ describe("scenarios > collection defaults", () => { pickEntity({ path: ["Our analytics", `Collection ${COLLECTIONS_COUNT}`], select: true, - tab: /Collections/, + tab: "Collections", }); cy.findByTestId("new-collection-modal").button("Create").click(); diff --git a/e2e/test/scenarios/collections/reproductions/24660-same-name-parent-collections.cy.spec.js b/e2e/test/scenarios/collections/reproductions/24660-same-name-parent-collections.cy.spec.js index 946322c06360f..5e67ebe610ff9 100644 --- a/e2e/test/scenarios/collections/reproductions/24660-same-name-parent-collections.cy.spec.js +++ b/e2e/test/scenarios/collections/reproductions/24660-same-name-parent-collections.cy.spec.js @@ -1,8 +1,13 @@ import { - ORDERS_QUESTION_ID, ORDERS_COUNT_QUESTION_ID, + ORDERS_QUESTION_ID, } from "e2e/support/cypress_sample_instance_data"; -import { restore, startNewQuestion } from "e2e/support/helpers"; +import { + entityPickerModal, + entityPickerModalTab, + restore, + startNewQuestion, +} from "e2e/support/helpers"; const collectionName = "Parent"; @@ -22,17 +27,13 @@ describe("issue 24660", () => { it("should properly show contents of different collections with the same name (metabase#24660)", () => { startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - cy.findAllByTestId("tree-item-name") - .contains(collectionName) - .first() - .click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findAllByText(collectionName).first().click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText(questions[ORDERS_QUESTION_ID]); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText(questions[ORDERS_COUNT_QUESTION_ID]).should("not.exist"); + cy.findByText(questions[ORDERS_QUESTION_ID]).should("exist"); + cy.findByText(questions[ORDERS_COUNT_QUESTION_ID]).should("not.exist"); + }); }); }); diff --git a/e2e/test/scenarios/collections/trash.cy.spec.js b/e2e/test/scenarios/collections/trash.cy.spec.js index 3bd4c38d35071..b64a12f9e2c30 100644 --- a/e2e/test/scenarios/collections/trash.cy.spec.js +++ b/e2e/test/scenarios/collections/trash.cy.spec.js @@ -14,6 +14,7 @@ import { modal, navigationSidebar, restore, + entityPickerModalTab, } from "e2e/support/helpers"; describe("scenarios > collections > trash", () => { @@ -83,11 +84,13 @@ describe("scenarios > collections > trash", () => { cy.findByLabelText("Navigation bar").within(() => { cy.findByText("New").click(); }); + popover().findByText("Question").click(); - popover().findByText("Models").click(); - popover().within(() => { + entityPickerModal().within(() => { + entityPickerModalTab("Models").click(); cy.findByText("Our analytics").should("exist"); cy.findByText("Trash").should("not.exist"); + cy.button("Close").click(); }); cy.log("trash should not appear in collection picker"); diff --git a/e2e/test/scenarios/custom-column/cc-data-type.cy.spec.js b/e2e/test/scenarios/custom-column/cc-data-type.cy.spec.js index 70f2f094b9da8..5ffb309e9f64e 100644 --- a/e2e/test/scenarios/custom-column/cc-data-type.cy.spec.js +++ b/e2e/test/scenarios/custom-column/cc-data-type.cy.spec.js @@ -1,5 +1,7 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { + entityPickerModal, + entityPickerModalTab, restore, openTable, popover, @@ -41,8 +43,8 @@ describe("scenarios > question > custom column > data type", () => { it("should understand date functions", () => { startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("QA Postgres12").click(); cy.findByText("Orders").click(); }); diff --git a/e2e/test/scenarios/custom-column/custom-column.cy.spec.js b/e2e/test/scenarios/custom-column/custom-column.cy.spec.js index 4c870a0f4f097..8a0420c613952 100644 --- a/e2e/test/scenarios/custom-column/custom-column.cy.spec.js +++ b/e2e/test/scenarios/custom-column/custom-column.cy.spec.js @@ -2,6 +2,7 @@ import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { addCustomColumn, + entityPickerModal, restore, popover, summarize, @@ -15,6 +16,7 @@ import { checkExpressionEditorHelperPopoverPosition, queryBuilderMain, cartesianChartCircle, + entityPickerModalTab, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE; @@ -247,8 +249,11 @@ describe("scenarios > question > custom column", () => { // join with Products // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Join data").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Products").click(); + + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Products").click(); + }); // add custom column // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage diff --git a/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js b/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js index 1b01a418c41a0..4425eefecf46e 100644 --- a/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js +++ b/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js @@ -2,12 +2,14 @@ import { addCustomColumn, enterCustomColumnDetails, getNotebookStep, + entityPickerModal, popover, visualize, restore, startNewQuestion, queryBuilderMain, selectFilterOperator, + entityPickerModalTab, } from "e2e/support/helpers"; const CC_NAME = "C-States"; @@ -19,9 +21,11 @@ describe("issue 13751", { tags: "@external" }, () => { cy.signInAsAdmin(); startNewQuestion(); - popover().findByText("Raw Data").click(); - popover().findByText(PG_DB_NAME).should("be.visible").click(); - popover().findByTextEnsureVisible("People").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText(PG_DB_NAME).should("be.visible").click(); + cy.findByTextEnsureVisible("People").click(); + }); }); it("should allow using strings in filter based on a custom column (metabase#13751)", () => { diff --git a/e2e/test/scenarios/custom-column/reproductions/14843-cc-apply-filter-not-equal-to.cy.spec.js b/e2e/test/scenarios/custom-column/reproductions/14843-cc-apply-filter-not-equal-to.cy.spec.js index cfbf34d0fc5c1..edd830b672f97 100644 --- a/e2e/test/scenarios/custom-column/reproductions/14843-cc-apply-filter-not-equal-to.cy.spec.js +++ b/e2e/test/scenarios/custom-column/reproductions/14843-cc-apply-filter-not-equal-to.cy.spec.js @@ -1,12 +1,11 @@ -import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { - restore, - popover, - visualize, filter, openNotebook, + popover, + restore, selectFilterOperator, + visualize, } from "e2e/support/helpers"; const { PEOPLE, PEOPLE_ID } = SAMPLE_DATABASE; @@ -23,9 +22,6 @@ const questionDetails = { describe("issue 14843", () => { beforeEach(() => { cy.intercept("POST", "/api/dataset").as("dataset"); - cy.intercept("GET", `/api/database/${SAMPLE_DB_ID}/schema/PUBLIC`).as( - "schema", - ); restore(); cy.signInAsAdmin(); @@ -35,8 +31,6 @@ describe("issue 14843", () => { cy.createQuestion(questionDetails, { visitQuestion: true }); openNotebook(); - cy.wait("@schema"); - filter({ mode: "notebook" }); popover().findByText(CC_NAME).click(); selectFilterOperator("Not equal to"); diff --git a/e2e/test/scenarios/custom-column/reproductions/21135-cc-same-name-as-existing-column.cy.spec.js b/e2e/test/scenarios/custom-column/reproductions/21135-cc-same-name-as-existing-column.cy.spec.js index bb9b3490c479f..2759a0a29c46b 100644 --- a/e2e/test/scenarios/custom-column/reproductions/21135-cc-same-name-as-existing-column.cy.spec.js +++ b/e2e/test/scenarios/custom-column/reproductions/21135-cc-same-name-as-existing-column.cy.spec.js @@ -1,4 +1,3 @@ -import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { restore } from "e2e/support/helpers"; @@ -17,10 +16,8 @@ describe("issue 21135", () => { beforeEach(() => { restore(); cy.signInAsAdmin(); - cy.createQuestion(questionDetails, { visitQuestion: true }); - - switchToNotebookView(); + cy.icon("notebook").click(); }); it("should handle cc with the same name as the table column (metabase#21135)", () => { @@ -42,15 +39,6 @@ describe("issue 21135", () => { }); }); -function switchToNotebookView() { - cy.intercept("GET", `/api/database/${SAMPLE_DB_ID}/schema/PUBLIC`).as( - "publicSchema", - ); - - cy.icon("notebook").click(); - cy.wait("@publicSchema"); -} - function previewCustomColumnNotebookStep() { cy.intercept("POST", "/api/dataset").as("dataset"); diff --git a/e2e/test/scenarios/custom-column/reproductions/27745-cc-numeric-missing-summarize.cy.spec.js b/e2e/test/scenarios/custom-column/reproductions/27745-cc-numeric-missing-summarize.cy.spec.js index 44d387772f86a..48138223eba4f 100644 --- a/e2e/test/scenarios/custom-column/reproductions/27745-cc-numeric-missing-summarize.cy.spec.js +++ b/e2e/test/scenarios/custom-column/reproductions/27745-cc-numeric-missing-summarize.cy.spec.js @@ -1,5 +1,6 @@ import { WRITABLE_DB_ID } from "e2e/support/cypress_data"; import { + entityPickerModal, restore, startNewQuestion, enterCustomColumnDetails, @@ -22,10 +23,13 @@ import { it("should display all summarize options if the only numeric field is a custom column (metabase#27745)", () => { startNewQuestion(); - cy.findByPlaceholderText(/Search for some data/).type("colors"); - popover() - .findByRole("heading", { name: /colors/i }) - .click(); + + entityPickerModal().within(() => { + cy.findByPlaceholderText("Search…").type("colors"); + cy.findByTestId("result-item") + .contains(/colors/i) + .click(); + }); cy.icon("add_data").click(); enterCustomColumnDetails({ formula: "case([ID] > 1, 25, 5)", diff --git a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js index 56fd9d4f745aa..7923f15dbbc17 100644 --- a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js +++ b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js @@ -114,8 +114,8 @@ describe("scenarios > dashboard", () => { .findByRole("link", { name: "ask a new one" }) .click(); - popover().within(() => { - cy.findByPlaceholderText(/Search for some/).type("Pro"); + entityPickerModal().within(() => { + cy.findByPlaceholderText("Search…").type("Pro"); cy.findByText("Products").click(); }); diff --git a/e2e/test/scenarios/embedding/interactive-embedding.cy.spec.js b/e2e/test/scenarios/embedding/interactive-embedding.cy.spec.js index 540f64ad3dbe7..da91ce980c745 100644 --- a/e2e/test/scenarios/embedding/interactive-embedding.cy.spec.js +++ b/e2e/test/scenarios/embedding/interactive-embedding.cy.spec.js @@ -5,6 +5,7 @@ import { } from "e2e/support/cypress_sample_instance_data"; import { adhocQuestionHash, + entityPickerModal, popover, appBar, restore, @@ -20,6 +21,7 @@ import { goToTab, visitFullAppEmbeddingUrl, getDashboardCardMenu, + entityPickerModalTab, } from "e2e/support/helpers"; import { createMockDashboardCard, @@ -238,8 +240,10 @@ describeEE("scenarios > embedding > full app", () => { cy.button("New").click(); popover().findByText("Question").click(); - popover().findByText("Raw Data").click(); - popover().findByText("Orders").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Orders").click(); + }); }); it("should show the database for a new native question (metabase#21511)", () => { diff --git a/e2e/test/scenarios/filters/reproductions/20683-postgres-current-quarter.cy.spec.js b/e2e/test/scenarios/filters/reproductions/20683-postgres-current-quarter.cy.spec.js index 1ea4ca80fe2b0..3a8d773bd76ee 100644 --- a/e2e/test/scenarios/filters/reproductions/20683-postgres-current-quarter.cy.spec.js +++ b/e2e/test/scenarios/filters/reproductions/20683-postgres-current-quarter.cy.spec.js @@ -1,10 +1,12 @@ import { + entityPickerModal, + entityPickerModalTab, + getNotebookStep, popover, + queryBuilderMain, restore, - visualize, startNewQuestion, - queryBuilderMain, - getNotebookStep, + visualize, } from "e2e/support/helpers"; describe("issue 20683", { tags: "@external" }, () => { @@ -15,8 +17,8 @@ describe("issue 20683", { tags: "@external" }, () => { it("should filter postgres with the 'current quarter' filter (metabase#20683)", () => { startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("QA Postgres12").click(); cy.findByText("Orders").click(); }); diff --git a/e2e/test/scenarios/filters/reproductions/29094-non-boolean-custom-expressions.cy.spec.js b/e2e/test/scenarios/filters/reproductions/29094-non-boolean-custom-expressions.cy.spec.js index 64a6181e1f635..c3af829300598 100644 --- a/e2e/test/scenarios/filters/reproductions/29094-non-boolean-custom-expressions.cy.spec.js +++ b/e2e/test/scenarios/filters/reproductions/29094-non-boolean-custom-expressions.cy.spec.js @@ -1,5 +1,7 @@ import { enterCustomColumnDetails, + entityPickerModal, + entityPickerModalTab, getNotebookStep, popover, restore, @@ -15,8 +17,8 @@ describe("issue 29094", () => { it("disallows adding a filter using non-boolean custom expression (metabase#29094)", () => { startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); diff --git a/e2e/test/scenarios/joins/joins.cy.spec.js b/e2e/test/scenarios/joins/joins.cy.spec.js index cd7a61da9c4f1..d2b6531504311 100644 --- a/e2e/test/scenarios/joins/joins.cy.spec.js +++ b/e2e/test/scenarios/joins/joins.cy.spec.js @@ -7,6 +7,7 @@ import { assertJoinValid, assertQueryBuilderRowCount, enterCustomColumnDetails, + entityPickerModal, filter, getNotebookStep, join, @@ -256,7 +257,7 @@ describe("scenarios > question > joined questions", () => { addSummaryGroupingField({ table: "Product", field: "ID" }); cy.findAllByTestId("action-buttons").last().button("Join data").click(); - joinTable("Reviews", "ID", "Product ID"); + joinTable("Reviews"); visualize(); assertJoinValid({ @@ -352,7 +353,7 @@ describe("scenarios > question > joined questions", () => { ); getNotebookStep("data").findByTestId("data-step-cell").click(); - popover().findByText("People").click(); + entityPickerModal().findByText("People").click(); getNotebookStep("join").should("not.exist"); diff --git a/e2e/test/scenarios/joins/reproductions/15342-mysql-correct-joins-order.cy.spec.js b/e2e/test/scenarios/joins/reproductions/15342-mysql-correct-joins-order.cy.spec.js index cec8bf9ddf7af..3f275094056fd 100644 --- a/e2e/test/scenarios/joins/reproductions/15342-mysql-correct-joins-order.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/15342-mysql-correct-joins-order.cy.spec.js @@ -1,8 +1,11 @@ import { - restore, + entityPickerModal, + entityPickerModalTab, + getNotebookStep, popover, - visualize, + restore, startNewQuestion, + visualize, } from "e2e/support/helpers"; const MYSQL_DB_NAME = "QA MySQL8"; @@ -17,22 +20,21 @@ describe("issue 15342", { tags: "@external" }, () => { it("should correctly order joins for MySQL queries (metabase#15342)", () => { startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText(MYSQL_DB_NAME).click(); cy.findByText("People").click(); }); - addJoin({ - leftColumn: "ID", - rightTable: "Orders", - rightColumn: "Product ID", - }); + cy.icon("join_left_outer").click(); + entityPickerModal().findByText("Orders").click(); + getNotebookStep("join").findByLabelText("Right column").click(); + popover().findByText("Product ID").click(); - addJoin({ - rightTable: "Products", - joinType: "inner", - }); + cy.icon("join_left_outer").last().click(); + entityPickerModal().findByText("Products").click(); + getNotebookStep("join").icon("join_left_outer").click(); + popover().findByText("Inner join").click(); visualize(); @@ -43,45 +45,3 @@ describe("issue 15342", { tags: "@external" }, () => { }); }); }); - -function selectFromDropdown(itemName) { - return popover().findByText(itemName); -} - -const JOIN_LABEL = { - left: "Left outer join", - right: "Right outer join", - inner: "Inner join", -}; - -function addJoin({ - leftTable, - leftColumn, - rightTable, - rightColumn, - joinType = "left", -} = {}) { - cy.icon("join_left_outer").last().click(); - - selectFromDropdown(rightTable).click(); - - if (leftTable) { - selectFromDropdown(leftTable).click(); - } - - if (leftColumn) { - selectFromDropdown(leftColumn).click(); - } - - if (rightColumn) { - selectFromDropdown(rightColumn).click(); - } - - cy.findAllByText("Join data") - .last() - .next() - .within(() => { - cy.icon("join_left_outer").click(); - }); - selectFromDropdown(JOIN_LABEL[joinType]).click(); -} diff --git a/e2e/test/scenarios/joins/reproductions/15578-remapped-values-joins.cy.spec.js b/e2e/test/scenarios/joins/reproductions/15578-remapped-values-joins.cy.spec.js index fb85c968fa874..6dd1682f28b90 100644 --- a/e2e/test/scenarios/joins/reproductions/15578-remapped-values-joins.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/15578-remapped-values-joins.cy.spec.js @@ -1,11 +1,12 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { - restore, - popover, - visualize, + entityPickerModal, + entityPickerModalTab, openProductsTable, queryBuilderHeader, queryBuilderMain, + restore, + visualize, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PRODUCTS } = SAMPLE_DATABASE; @@ -35,11 +36,10 @@ describe("issue 15578", () => { openProductsTable({ mode: "notebook" }); cy.button("Join data").click(); - - popover().findByText("Sample Database").click(); - popover().findByText("Raw Data").click(); - popover().findByText("Saved Questions").click(); - popover().findByText(JOINED_QUESTION_NAME).click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText(JOINED_QUESTION_NAME).click(); + }); visualize(); diff --git a/e2e/test/scenarios/joins/reproductions/17710-notebook-incomplete-joins-removed.cy.spec.js b/e2e/test/scenarios/joins/reproductions/17710-notebook-incomplete-joins-removed.cy.spec.js index 7b589d772585a..eafa254a11563 100644 --- a/e2e/test/scenarios/joins/reproductions/17710-notebook-incomplete-joins-removed.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/17710-notebook-incomplete-joins-removed.cy.spec.js @@ -1,7 +1,9 @@ import { - restore, - popover, + entityPickerModal, + entityPickerModalTab, + getNotebookStep, openOrdersTable, + restore, visualize, } from "e2e/support/helpers"; @@ -15,14 +17,16 @@ describe("issue 17710", () => { it("should remove only invalid join clauses (metabase#17710)", () => { openOrdersTable({ mode: "notebook" }); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Join data").click(); - popover().findByText("Products").click(); + cy.button("Join data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Products").click(); + }); - cy.findByTestId("step-join-0-0").icon("add").click(); + getNotebookStep("join").icon("add").click(); // Close the LHS column popover that opens automatically - cy.findByTestId("step-join-0-0").parent().click(); + getNotebookStep("join").parent().click(); visualize(); diff --git a/e2e/test/scenarios/joins/reproductions/17767-cannot-join-on-aggregation-with-implicit-joins.cy.spec.js b/e2e/test/scenarios/joins/reproductions/17767-cannot-join-on-aggregation-with-implicit-joins.cy.spec.js index cb1c82bc05742..fafda3eb04e87 100644 --- a/e2e/test/scenarios/joins/reproductions/17767-cannot-join-on-aggregation-with-implicit-joins.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/17767-cannot-join-on-aggregation-with-implicit-joins.cy.spec.js @@ -1,5 +1,10 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; -import { restore, popover, visualize } from "e2e/support/helpers"; +import { + entityPickerModal, + entityPickerModalTab, + restore, + visualize, +} from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PRODUCTS } = SAMPLE_DATABASE; @@ -30,14 +35,10 @@ describe("issue 17767", () => { cy.findByText("Join data").click(); // Join "Previous results" with - popover().contains("Reviews").click(); - - // On - popover().contains("ID").click(); - // = - popover() - .contains(/Products? ID/) - .click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Reviews").click(); + }); visualize(response => { expect(response.body.error).to.not.exist; diff --git a/e2e/test/scenarios/joins/reproductions/17968-notebook-join-table-names.cy.spec.js b/e2e/test/scenarios/joins/reproductions/17968-notebook-join-table-names.cy.spec.js index 96fedb67b5710..fcd8f67f05cf1 100644 --- a/e2e/test/scenarios/joins/reproductions/17968-notebook-join-table-names.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/17968-notebook-join-table-names.cy.spec.js @@ -1,8 +1,10 @@ import { - restore, + entityPickerModal, + entityPickerModalTab, getNotebookStep, openOrdersTable, popover, + restore, summarize, } from "e2e/support/helpers"; @@ -24,7 +26,10 @@ describe("issue 17968", () => { popover().findByText("Created At").click(); cy.findAllByTestId("action-buttons").last().button("Join data").click(); - popover().findByText("Products").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Products").click(); + }); popover().findByText("Count").click(); getNotebookStep("join", { stage: 1 }) diff --git a/e2e/test/scenarios/joins/reproductions/18589-numeric-binning-in-joins.cy.spec.js b/e2e/test/scenarios/joins/reproductions/18589-numeric-binning-in-joins.cy.spec.js index fffd4ab9c59e4..39d38d6e6d775 100644 --- a/e2e/test/scenarios/joins/reproductions/18589-numeric-binning-in-joins.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/18589-numeric-binning-in-joins.cy.spec.js @@ -1,9 +1,11 @@ import { - restore, + entityPickerModal, + entityPickerModalTab, openOrdersTable, - visualize, popover, + restore, summarize, + visualize, } from "e2e/support/helpers"; describe("issue 18589", () => { @@ -32,7 +34,10 @@ describe("issue 18589", () => { function joinTable(table) { cy.findByText("Join data").click(); - popover().findByText(table).click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText(table).click(); + }); } function selectFromDropdown(option, clickOpts) { diff --git a/e2e/test/scenarios/joins/reproductions/20519-cannot-join-on-aggregation-with-implicit-joins-and-nested-query.cy.spec.js b/e2e/test/scenarios/joins/reproductions/20519-cannot-join-on-aggregation-with-implicit-joins-and-nested-query.cy.spec.js index d11c065976acf..47e6d3a1ccf43 100644 --- a/e2e/test/scenarios/joins/reproductions/20519-cannot-join-on-aggregation-with-implicit-joins-and-nested-query.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/20519-cannot-join-on-aggregation-with-implicit-joins-and-nested-query.cy.spec.js @@ -1,4 +1,3 @@ -import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { restore, @@ -41,7 +40,7 @@ describe("issue 20519", () => { cy.signInAsAdmin(); cy.createQuestion(questionDetails, { visitQuestion: true }); - switchToNotebookView(); + cy.icon("notebook").click(); }); // Tightly related issue: metabase#17767 @@ -67,12 +66,3 @@ describe("issue 20519", () => { cy.contains("Two"); }); }); - -function switchToNotebookView() { - cy.intercept("GET", `/api/database/${SAMPLE_DB_ID}/schema/PUBLIC`).as( - "publicSchema", - ); - - cy.icon("notebook").click(); - cy.wait("@publicSchema"); -} diff --git a/e2e/test/scenarios/joins/reproductions/22859-multi-nested-joins-wrong-aliasing.cy.spec.js b/e2e/test/scenarios/joins/reproductions/22859-multi-nested-joins-wrong-aliasing.cy.spec.js index afc2e6440b00b..0279e73e19353 100644 --- a/e2e/test/scenarios/joins/reproductions/22859-multi-nested-joins-wrong-aliasing.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/22859-multi-nested-joins-wrong-aliasing.cy.spec.js @@ -1,9 +1,10 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { + entityPickerModal, + entityPickerModalTab, restore, - popover, - visualize, startNewQuestion, + visualize, } from "e2e/support/helpers"; const { REVIEWS, REVIEWS_ID, PRODUCTS, PRODUCTS_ID, ORDERS_ID, ORDERS } = @@ -86,8 +87,8 @@ describe("issue 22859 - multiple levels of nesting", () => { it("third level of nesting with joins should result in proper column aliasing (metabase#22859-2)", () => { startNewQuestion(); - popover().within(() => { - cy.findByText("Saved Questions").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); cy.findByText("22859-Q2").click(); }); diff --git a/e2e/test/scenarios/joins/reproductions/29795-native-query-join-question-misses-metadata.cy.spec.js b/e2e/test/scenarios/joins/reproductions/29795-native-query-join-question-misses-metadata.cy.spec.js index 0608cd6f2933b..517532e8c7b84 100644 --- a/e2e/test/scenarios/joins/reproductions/29795-native-query-join-question-misses-metadata.cy.spec.js +++ b/e2e/test/scenarios/joins/reproductions/29795-native-query-join-question-misses-metadata.cy.spec.js @@ -1,8 +1,10 @@ import { + entityPickerModal, + entityPickerModalTab, + openOrdersTable, + popover, restore, visualize, - popover, - openOrdersTable, } from "e2e/support/helpers"; describe("issue 29795", () => { @@ -26,11 +28,9 @@ describe("issue 29795", () => { cy.icon("join_left_outer").click(); - popover().within(() => { - cy.icon("chevronleft").click(); - cy.findByText("Raw Data").click(); - cy.findByText("Saved Questions").click(); - cy.findByRole("menuitem", { name: NATIVE_QUESTION }).click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText(NATIVE_QUESTION).click(); }); popover().within(() => { diff --git a/e2e/test/scenarios/models/create.cy.spec.js b/e2e/test/scenarios/models/create.cy.spec.js index 91a7747c01e14..21e263b8bd72a 100644 --- a/e2e/test/scenarios/models/create.cy.spec.js +++ b/e2e/test/scenarios/models/create.cy.spec.js @@ -1,5 +1,11 @@ import { THIRD_COLLECTION_ID } from "e2e/support/cypress_sample_instance_data"; -import { modal, popover, restore, visitCollection } from "e2e/support/helpers"; +import { + entityPickerModal, + entityPickerModalTab, + modal, + restore, + visitCollection, +} from "e2e/support/helpers"; const modelName = "A name"; @@ -67,8 +73,8 @@ describe("scenarios > models > create", () => { navigateToNewModelPage("structured"); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); diff --git a/e2e/test/scenarios/models/helpers/e2e-models-helpers.js b/e2e/test/scenarios/models/helpers/e2e-models-helpers.js index 182b51c3d2915..0b9ecb967a5b9 100644 --- a/e2e/test/scenarios/models/helpers/e2e-models-helpers.js +++ b/e2e/test/scenarios/models/helpers/e2e-models-helpers.js @@ -1,8 +1,10 @@ import { - popover, + entityPickerModal, + entityPickerModalTab, + interceptIfNotPreviouslyDefined, modal, openQuestionActions, - interceptIfNotPreviouslyDefined, + popover, } from "e2e/support/helpers"; export function assertQuestionIsBasedOnModel({ @@ -105,9 +107,9 @@ export function selectFromDropdown(option, clickOpts) { export function startQuestionFromModel(modelName) { cy.findByTestId("app-bar").findByText("New").click(); - popover().within(() => { - cy.findByText("Question").should("be.visible").click(); - cy.findByRole("menuitem", { name: /Models/ }).click(); - cy.findByRole("menuitem", { name: modelName }).click(); + popover().findByText("Question").should("be.visible").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Models").click(); + cy.findByText(modelName).click(); }); } diff --git a/e2e/test/scenarios/models/models.cy.spec.js b/e2e/test/scenarios/models/models.cy.spec.js index ccfad1f590195..b5c0b755368d5 100644 --- a/e2e/test/scenarios/models/models.cy.spec.js +++ b/e2e/test/scenarios/models/models.cy.spec.js @@ -30,8 +30,10 @@ import { selectFilterOperator, focusNativeEditor, echartsContainer, + entityPickerModal, + questionInfoButton, + entityPickerModalTab, } from "e2e/support/helpers"; -import { questionInfoButton } from "e2e/support/helpers/e2e-ui-elements-helpers"; import { turnIntoModel, @@ -243,45 +245,32 @@ describe("scenarios > models", () => { it("transforms the data picker", () => { startNewQuestion(); - popover().within(() => { - testDataPickerSearch({ - inputPlaceholderText: "Search for some data…", - query: "Ord", - models: true, - cards: true, - tables: true, - }); + entityPickerModal().within(() => { + entityPickerModalTab("Models").click(); + cy.findByText("Orders").should("exist"); + cy.findByText("Orders Model").should("exist"); + cy.findByText("Orders, Count").should("not.exist"); + + entityPickerModalTab("Saved questions").click(); + cy.findByText("Orders").should("not.exist"); + cy.findByText("Orders Model").should("not.exist"); + cy.findByText("Orders, Count").should("exist"); + cy.findByText("Orders, Count, Grouped by Created At (year)").should( + "exist", + ); + cy.findByText("Products").should("exist"); + + entityPickerModalTab("Tables").click(); + cy.findByText("Orders").should("exist"); + cy.findByText("People").should("exist"); + cy.findByText("Products").should("exist"); + cy.findByText("Reviews").should("exist"); + cy.findByText("Orders, Count").should("not.exist"); - cy.findByText("Models").click(); - cy.findByTestId("select-list").within(() => { - cy.findByText("Orders"); - cy.findByText("Orders, Count").should("not.exist"); - }); testDataPickerSearch({ - inputPlaceholderText: "Search for a model…", query: "Ord", models: true, - }); - cy.icon("chevronleft").click(); - - cy.findByText("Saved Questions").click(); - cy.findByTestId("select-list").within(() => { - cy.findByText("Orders, Count"); - cy.findByText("Orders").should("not.exist"); - }); - testDataPickerSearch({ - inputPlaceholderText: "Search for a question…", - query: "Ord", cards: true, - }); - cy.icon("chevronleft").click(); - - cy.findByText("Raw Data").click(); - cy.findByText("Sample Database").click(); // go back to db list - cy.findByText("Saved Questions").should("not.exist"); - testDataPickerSearch({ - inputPlaceholderText: "Search for a table…", - query: "Ord", tables: true, }); }); @@ -289,19 +278,23 @@ describe("scenarios > models", () => { it("allows to create a question based on a model", () => { cy.intercept(`/api/database/${SAMPLE_DB_ID}/schema/PUBLIC`).as("schema"); - startNewQuestion(); - popover().within(() => { - cy.findByText("Models").click(); + startNewQuestion(); + entityPickerModal().within(() => { + entityPickerModalTab("Models").click(); cy.findByText("Orders").click(); }); cy.icon("join_left_outer").click(); - selectFromDropdown("Models"); - selectFromDropdown("Raw Data"); - selectFromDropdown("Sample Database"); - cy.findAllByRole("option").should("have.length", 4); - selectFromDropdown("Products"); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Orders").should("exist"); + cy.findByText("People").should("exist"); + cy.findByText("Products").should("exist"); + cy.findByText("Reviews").should("exist"); + + cy.findByText("Products").click(); + }); getNotebookStep("filter") .findByText("Add filters to narrow your answer") @@ -339,9 +332,13 @@ describe("scenarios > models", () => { it("should not display models if nested queries are disabled", () => { mockSessionProperty("enable-nested-queries", false); startNewQuestion(); - popover().within(() => { - cy.findByText("Models").should("not.exist"); - cy.findByText("Saved Questions").should("not.exist"); + entityPickerModal().within(() => { + cy.findAllByRole("tab").should("not.exist"); + + cy.findByText("Orders").should("exist"); + cy.findByText("People").should("exist"); + cy.findByText("Products").should("exist"); + cy.findByText("Reviews").should("exist"); }); }); }); @@ -612,16 +609,15 @@ function getCollectionItemCard(itemName) { } function testDataPickerSearch({ - inputPlaceholderText, query, models = false, cards = false, tables = false, } = {}) { - cy.findByPlaceholderText(inputPlaceholderText).type(query); + cy.findByPlaceholderText("Search…").type(query); cy.wait("@search"); - const searchResultItems = cy.findAllByTestId("search-result-item"); + const searchResultItems = cy.findAllByTestId("result-item"); searchResultItems.then($results => { const modelTypes = {}; @@ -653,6 +649,4 @@ function testDataPickerSearch({ expect(Object.keys(modelTypes)).not.to.include("table"); } }); - - cy.icon("close").click(); } diff --git a/e2e/test/scenarios/models/reproductions/19737-data-picker-not-showing-moved-model.cy.spec.js b/e2e/test/scenarios/models/reproductions/19737-data-picker-not-showing-moved-model.cy.spec.js index 6b215d10e8e33..31849fcdf41b1 100644 --- a/e2e/test/scenarios/models/reproductions/19737-data-picker-not-showing-moved-model.cy.spec.js +++ b/e2e/test/scenarios/models/reproductions/19737-data-picker-not-showing-moved-model.cy.spec.js @@ -1,9 +1,10 @@ import { - restore, - popover, + entityPickerModal, + entityPickerModalLevel, navigationSidebar, openNavigationSidebar, - entityPickerModal, + popover, + restore, } from "e2e/support/helpers"; const modelName = "Orders Model"; @@ -28,9 +29,8 @@ describe("issue 19737", () => { // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Question").should("be.visible").click(); - popover().within(() => { - cy.findByText("Models").click(); - cy.findByText("Your personal collection").click(); + entityPickerModal().within(() => { + cy.findByText(personalCollectionName).click(); cy.findByText(modelName); }); }); @@ -52,8 +52,7 @@ describe("issue 19737", () => { cy.findByText("Question").should("be.visible").click(); // Open question picker (this is crucial) so the collection list are loaded. - popover().within(() => { - cy.findByText("Models").click(); + entityPickerModal().within(() => { cy.findByText("First collection").click(); cy.findByText(modelName); }); @@ -75,10 +74,10 @@ describe("issue 19737", () => { // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Question").should("be.visible").click(); - popover().within(() => { - cy.findByText("Models").click(); - cy.findByText("First collection").click(); - cy.findByText("Nothing here"); + entityPickerModal().within(() => { + cy.findByText("First collection").should("not.exist"); + entityPickerModalLevel(1).should("not.exist"); + entityPickerModalLevel(2).should("not.exist"); }); }); }); diff --git a/e2e/test/scenarios/models/reproductions/25537-model-picker-locale.cy.spec.js b/e2e/test/scenarios/models/reproductions/25537-model-picker-locale.cy.spec.js index cc5efbe2840f3..c31858cc32b7d 100644 --- a/e2e/test/scenarios/models/reproductions/25537-model-picker-locale.cy.spec.js +++ b/e2e/test/scenarios/models/reproductions/25537-model-picker-locale.cy.spec.js @@ -1,5 +1,9 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; -import { restore, startNewQuestion } from "e2e/support/helpers"; +import { + entityPickerModal, + restore, + startNewQuestion, +} from "e2e/support/helpers"; const { ORDERS_ID } = SAMPLE_DATABASE; @@ -21,11 +25,11 @@ describe("issue 25537", () => { cy.createQuestion(questionDetails); startNewQuestion(); - cy.findByRole("menuitem", { name: /Modelle/i }).click(); - cy.wait("@getCollectionContent"); - - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText(questionDetails.name).should("exist"); + entityPickerModal().within(() => { + cy.findByText(/Modelle/i).click(); + cy.wait("@getCollectionContent"); + cy.findByText(questionDetails.name).should("exist"); + }); }); }); diff --git a/e2e/test/scenarios/models/reproductions/26091-new-models-picker.cy.spec.js b/e2e/test/scenarios/models/reproductions/26091-new-models-picker.cy.spec.js index 7d5843e7a5691..a8a191afa6cda 100644 --- a/e2e/test/scenarios/models/reproductions/26091-new-models-picker.cy.spec.js +++ b/e2e/test/scenarios/models/reproductions/26091-new-models-picker.cy.spec.js @@ -1,5 +1,10 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; -import { popover, restore } from "e2e/support/helpers"; +import { + entityPickerModal, + entityPickerModalTab, + popover, + restore, +} from "e2e/support/helpers"; import { turnIntoModel } from "../helpers/e2e-models-helpers"; @@ -15,7 +20,6 @@ describe("issue 26091", () => { beforeEach(() => { restore(); cy.signInAsAdmin(); - cy.intercept("GET", "/api/collection/*/items?*").as("getCollectionContent"); cy.intercept("POST", "/api/card").as("saveQuestion"); }); @@ -24,14 +28,8 @@ describe("issue 26091", () => { cy.visit("/"); startNewQuestion(); - popover().within(() => { - cy.findByText("Models").click(); - cy.wait("@getCollectionContent"); - }); - - startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage @@ -47,10 +45,20 @@ describe("issue 26091", () => { turnIntoModel(); startNewQuestion(); - popover().within(() => { - cy.findByText("Models").click(); - cy.findByText("Old model").should("be.visible"); + entityPickerModal().within(() => { + entityPickerModalTab("Recents").should( + "have.attr", + "aria-selected", + "true", + ); cy.findByText("New model").should("be.visible"); + cy.findByText("Old model").should("not.exist"); + cy.findByText("Orders Model").should("not.exist"); + + entityPickerModalTab("Models").click(); + cy.findByText("New model").should("be.visible"); + cy.findByText("Old model").should("be.visible"); + cy.findByText("Orders Model").should("be.visible"); }); }); }); diff --git a/e2e/test/scenarios/models/reproductions/28971-filters-model-new-model.cy.spec.js b/e2e/test/scenarios/models/reproductions/28971-filters-model-new-model.cy.spec.js index 9e79ff4c29cb2..afc4321c04a6a 100644 --- a/e2e/test/scenarios/models/reproductions/28971-filters-model-new-model.cy.spec.js +++ b/e2e/test/scenarios/models/reproductions/28971-filters-model-new-model.cy.spec.js @@ -1,4 +1,6 @@ import { + entityPickerModal, + entityPickerModalTab, filter, filterField, filterFieldPopover, @@ -22,8 +24,8 @@ describe("issue 28971", () => { popover().findByText("Model").click(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Use the notebook editor").click(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); cy.button("Save").click(); diff --git a/e2e/test/scenarios/models/reproductions/29951-model-editor-results-metadata.cy.spec.js b/e2e/test/scenarios/models/reproductions/29951-model-editor-results-metadata.cy.spec.js index eda7e927cd529..69eac4052a814 100644 --- a/e2e/test/scenarios/models/reproductions/29951-model-editor-results-metadata.cy.spec.js +++ b/e2e/test/scenarios/models/reproductions/29951-model-editor-results-metadata.cy.spec.js @@ -1,4 +1,3 @@ -import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { getNotebookStep, @@ -26,15 +25,11 @@ describe("issue 29951", { requestTimeout: 10000, viewportWidth: 1600 }, () => { restore(); cy.signInAsAdmin(); cy.intercept("PUT", "/api/card/*").as("updateCard"); - cy.intercept("GET", `/api/database/${SAMPLE_DB_ID}/schema/PUBLIC`).as( - "publicShema", - ); }); it("should allow to run the model query after changing custom columns (metabase#29951)", () => { cy.createQuestion(questionDetails).then(({ body: { id } }) => { cy.visit(`/model/${id}/query`); - cy.wait("@publicShema"); }); removeExpression("CC2"); diff --git a/e2e/test/scenarios/onboarding/reference/databases.cy.spec.js b/e2e/test/scenarios/onboarding/reference/databases.cy.spec.js index 3e41f41144ab2..fe47713963abb 100644 --- a/e2e/test/scenarios/onboarding/reference/databases.cy.spec.js +++ b/e2e/test/scenarios/onboarding/reference/databases.cy.spec.js @@ -1,4 +1,10 @@ -import { popover, restore, startNewQuestion } from "e2e/support/helpers"; +import { + entityPickerModal, + entityPickerModalTab, + popover, + restore, + startNewQuestion, +} from "e2e/support/helpers"; describe("scenarios > reference > databases", () => { beforeEach(() => { @@ -81,7 +87,15 @@ describe("scenarios > reference > databases", () => { }); it("should sort databases in new UI based question data selection popover", () => { - checkQuestionSourceDatabasesOrder(); + startNewQuestion(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.get("[data-index='0']").should("have.text", "a"); + cy.get("[data-index='1']").should("have.text", "b"); + cy.get("[data-index='2']").should("have.text", "c"); + cy.get("[data-index='3']").should("have.text", "d"); + cy.get("[data-index='4']").should("have.text", "Sample Database"); + }); }); it.skip("should sort databases in new native question data selection popover", () => { @@ -95,13 +109,10 @@ function checkReferenceDatabasesOrder() { cy.get("@databaseCard").last().should("have.text", "Sample Database"); } -function checkQuestionSourceDatabasesOrder(question_type) { +function checkQuestionSourceDatabasesOrder() { // Last item is "Saved Questions" for UI based questions so we have to check for the one before that (-2), and the last one for "Native" (-1) const lastDatabaseIndex = -1; - const selector = - question_type === "Native query" - ? "[data-element-id=list-item]-title" - : "[data-element-id=list-section-title]"; + const selector = "[data-element-id=list-item]-title"; startNewQuestion(); popover().within(() => { diff --git a/e2e/test/scenarios/permissions/sandboxes.cy.spec.js b/e2e/test/scenarios/permissions/sandboxes.cy.spec.js index 9951d56e80baa..1228ecf2cce31 100644 --- a/e2e/test/scenarios/permissions/sandboxes.cy.spec.js +++ b/e2e/test/scenarios/permissions/sandboxes.cy.spec.js @@ -26,6 +26,7 @@ import { selectFilterOperator, entityPickerModal, chartPathWithFillColor, + entityPickerModalTab, } from "e2e/support/helpers"; const { @@ -826,10 +827,10 @@ describeEE("formatting > sandboxes", () => { createJoinedQuestion("14766_joined"); startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("14766_joined").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("14766_joined").click(); + }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Pick the metric you want to see").click(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage diff --git a/e2e/test/scenarios/question/nested.cy.spec.js b/e2e/test/scenarios/question/nested.cy.spec.js index 9ebb6a197cff7..2dcae74e3ce12 100644 --- a/e2e/test/scenarios/question/nested.cy.spec.js +++ b/e2e/test/scenarios/question/nested.cy.spec.js @@ -12,6 +12,8 @@ import { filter, filterField, chartPathWithFillColor, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; import { createMetric } from "e2e/support/helpers/e2e-table-metadata-helpers"; @@ -395,10 +397,10 @@ describe("scenarios > question > nested", () => { cy.findByText("New").click(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Question").should("be.visible").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("15725").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("15725").click(); + }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Pick the metric you want to see").click(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage diff --git a/e2e/test/scenarios/question/new.cy.spec.js b/e2e/test/scenarios/question/new.cy.spec.js index fa41e683e28bb..93e6d67b848c6 100644 --- a/e2e/test/scenarios/question/new.cy.spec.js +++ b/e2e/test/scenarios/question/new.cy.spec.js @@ -20,10 +20,11 @@ import { describeOSS, queryBuilderHeader, entityPickerModal, + entityPickerModalItem, + entityPickerModalTab, collectionOnTheGoModal, modal, pickEntity, - hovercard, visitQuestion, } from "e2e/support/helpers"; @@ -45,9 +46,9 @@ describe("scenarios > question > new", () => { } startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); - cy.findByText("Sample3").isVisibleInPopover(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Sample3").should("be.visible"); }); }); @@ -58,72 +59,67 @@ describe("scenarios > question > new", () => { startNewQuestion(); - popover() - .findAllByRole("menuitem") - .should("have.length", 3) - .and("contain", "Models") - .and("contain", "Raw Data") - .and("contain", "Saved Questions"); - - // should not trigger search for an empty string - cy.findByPlaceholderText("Search for some data…").type(" ").blur(); - cy.findByPlaceholderText("Search for some data…").type("ord"); - cy.wait("@search"); - cy.get("@searchQuery").should("have.been.calledOnce"); - - // Search results include both saved questions and database tables - cy.findAllByTestId("search-result-item").should( - "have.length.at.least", - 4, - ); + entityPickerModal().within(() => { + cy.findAllByRole("tab").should("have.length", 3); + entityPickerModalTab("Models").should( + "have.attr", + "aria-selected", + "true", + ); + entityPickerModalTab("Tables").should("exist"); + entityPickerModalTab("Saved questions").should("exist"); + entityPickerModalTab("Search").should("not.exist"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Our analytics"); + cy.findByPlaceholderText("Search…").type(" ").blur(); + cy.findByPlaceholderText("Search…").type("ord"); + cy.wait("@search"); + // should not trigger search for an empty string + cy.get("@searchQuery").should("have.been.calledOnce"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Sample Database"); + cy.findAllByTestId("result-item").should("have.length.at.least", 4); - // Discarding the search query should take us back to the original selector - // that starts with the list of databases and saved questions - cy.findByPlaceholderText("Search for some data…"); - cy.findByTestId("input-reset-button").click(); + const searchResultItems = cy.findAllByTestId("result-item"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - - // Search is now scoped to questions only - cy.findByPlaceholderText("Search for a question…"); - cy.findByTestId("select-list") - .as("rightSide") - // should display the collection tree on the left side - .should("contain", "Orders") - .and("contain", "Orders, Count"); - - cy.get("@rightSide") - .siblings() - .should("have.length", 1) - .as("leftSide") - // should display the collection tree on the left side - .should("contain", "Our analytics"); + searchResultItems.then($results => { + const types = $results + .toArray() + .map(element => element.getAttribute("data-model-type")); + + // Search results include both saved questions and database tables + expect(types).to.include("card"); + expect(types).to.include("dataset"); + expect(types).to.include("table"); + }); + + // Discarding the search query should take us back to the original tab + cy.findByPlaceholderText("Search…").clear().blur(); + entityPickerModalTab("Search").should("not.exist"); + entityPickerModalTab("Models").should( + "have.attr", + "aria-selected", + "true", + ); + entityPickerModalTab("Saved questions").click(); + cy.findByText("Orders, Count").click(); + }); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Orders, Count").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Orders").should("not.exist"); visualize(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("18,760"); // should reopen saved question picker after returning back to editor mode cy.icon("notebook").click(); cy.findByTestId("data-step-cell").contains("Orders, Count").click(); - // It is now possible to choose another saved question - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Orders"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - popover().within(() => { - cy.contains("Raw Data").click(); - cy.contains("Sample Database").click(); + entityPickerModal().within(() => { + // It is now possible to choose another saved question + entityPickerModalTab("Saved questions").should( + "have.attr", + "aria-selected", + "true", + ); + cy.findByText("Orders").should("exist"); + cy.findByText("Orders, Count").should("exist"); + + entityPickerModalTab("Tables").click(); cy.findByText("Products").click(); }); cy.findByTestId("data-step-cell").contains("Products"); @@ -147,8 +143,8 @@ describe("scenarios > question > new", () => { }); startNewQuestion(); - popover().within(() => { - cy.findByText("Saved Questions").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); // Note: collection name's first letter is capitalized cy.findByText(/foo:bar/i).click(); cy.findByText("Orders"); @@ -163,10 +159,24 @@ describe("scenarios > question > new", () => { startNewQuestion(); - popover().within(() => { - cy.findByText("Saved Questions").click(); - cy.findByText("First collection"); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + assertDataPickerEntitySelected(0, "Our analytics"); + cy.findByText("First collection").should("exist"); cy.findByText("Second collection").should("not.exist"); + cy.findByText("Third collection").should("not.exist"); + + cy.findByText("First collection").click(); + assertDataPickerEntitySelected(0, "Our analytics"); + assertDataPickerEntitySelected(1, "First collection"); + cy.findByText("Second collection").should("exist"); + cy.findByText("Third collection").should("not.exist"); + + cy.findByText("Second collection").click(); + assertDataPickerEntitySelected(0, "Our analytics"); + assertDataPickerEntitySelected(1, "First collection"); + assertDataPickerEntitySelected(2, "Second collection"); + cy.findByText("Third collection").should("not.exist"); }); }); @@ -174,38 +184,21 @@ describe("scenarios > question > new", () => { cy.signOut(); cy.signIn("nocollection"); startNewQuestion(); - popover().within(() => { - cy.findByText("Orders").click(); - }); + entityPickerModal().findByText("Orders").click(); visualize(); saveQuestion("Personal question"); cy.signOut(); cy.signInAsAdmin(); startNewQuestion(); - popover().within(() => { - cy.findByText("Saved Questions").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); cy.findByText("All personal collections").click(); - cy.findByText(getPersonalCollectionName(USERS.normal)).should( - "not.exist", - ); cy.findByText(getPersonalCollectionName(USERS.nocollection)).click(); cy.findByText("Personal question").click(); }); visualize(); }); - - it("should allow clicking linked tables in table info popover", () => { - startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); - cy.findByLabelText("People").findByLabelText("More info").realHover(); - }); - - hovercard().findByText("Orders").click(); - - cy.url().should("include", "question#"); - }); }); it("composite keys should act as filters on click (metabase#13717)", () => { @@ -272,8 +265,8 @@ describe("scenarios > question > new", () => { popover().findByText("Question").click(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); @@ -293,8 +286,8 @@ describe("scenarios > question > new", () => { cy.findByLabelText("Navigation bar").findByText("New").click(); popover().findByText("Question").click(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); cy.findByTestId("qb-header").findByText("Save").click(); @@ -313,8 +306,10 @@ describe("scenarios > question > new", () => { cy.findByLabelText(/Give it a name/).type(NEW_COLLECTION); cy.findByText("Create").click(); }); - entityPickerModal().findByText("Foo").click(); - entityPickerModal().findByText("Select").click(); + entityPickerModal().within(() => { + cy.findByText("Foo").click(); + cy.findByText("Select").click(); + }); cy.findByTestId("save-question-modal").within(() => { cy.findByText("Save new question"); cy.findByLabelText(/Which collection/).should( @@ -377,8 +372,8 @@ describe("scenarios > question > new", () => { }); it("should hide public collections when selecting a dashboard for a question in a personal collection", () => { - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); @@ -390,7 +385,7 @@ describe("scenarios > question > new", () => { pickEntity({ path: [myPersonalCollectionName], select: true, - tab: /Collections/, + tab: "Collections", }); cy.findByTestId("save-question-modal").button("Save").click(); @@ -413,8 +408,8 @@ describe("scenarios > question > new", () => { }); it("should show all collections when selecting a dashboard for a question in a public collection", () => { - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); @@ -508,3 +503,7 @@ describeOSS( }); }, ); + +function assertDataPickerEntitySelected(level, name) { + entityPickerModalItem(level, name).should("have.attr", "data-active", "true"); +} diff --git a/e2e/test/scenarios/question/notebook-data-source.cy.spec.ts b/e2e/test/scenarios/question/notebook-data-source.cy.spec.ts index e8550d8fd84ea..cc1e2a9d80e83 100644 --- a/e2e/test/scenarios/question/notebook-data-source.cy.spec.ts +++ b/e2e/test/scenarios/question/notebook-data-source.cy.spec.ts @@ -13,6 +13,9 @@ import type { StructuredQuestionDetails } from "e2e/support/helpers"; import { createQuestion, entityPickerModal, + entityPickerModalItem, + entityPickerModalLevel, + entityPickerModalTab, isEE, isOSS, openNotebook, @@ -52,16 +55,23 @@ describe("scenarios > notebook > data source", () => { "Pick your starting data", ); - popover().within(() => { - cy.findByTestId("source-database").should( - "have.text", - "Sample Database", - ); - cy.findAllByRole("option") - .should("have.length", 8) - .each(table => { - cy.wrap(table).should("have.attr", "aria-selected", "false"); - }); + entityPickerModal().within(() => { + cy.log("Should not have Recents tab"); + cy.findAllByRole("tab").should("have.length", 0); + + entityPickerModalLevel(0).should("not.exist"); + entityPickerModalLevel(1).should("not.exist"); + entityPickerModalLevel(2) + .get("[data-index]") + .should("have.length", 8); + assertDataPickerEntityNotSelected(2, "Accounts"); + assertDataPickerEntityNotSelected(2, "Analytic Events"); + assertDataPickerEntityNotSelected(2, "Feedback"); + assertDataPickerEntityNotSelected(2, "Invoices"); + assertDataPickerEntityNotSelected(2, "Orders"); + assertDataPickerEntityNotSelected(2, "People"); + assertDataPickerEntityNotSelected(2, "Products"); + assertDataPickerEntityNotSelected(2, "Reviews"); }); }, ); @@ -76,16 +86,21 @@ describe("scenarios > notebook > data source", () => { "Pick your starting data", ); - popover().within(() => { - cy.findByTestId("source-database").should( - "have.text", - "Sample Database", - ); - cy.findAllByRole("option") - .should("have.length", 8) - .each(table => { - cy.wrap(table).should("have.attr", "aria-selected", "false"); - }); + entityPickerModal().within(() => { + cy.log("Should not have Recents tab"); + cy.findAllByRole("tab").should("have.length", 0); + + entityPickerModalLevel(0).should("not.exist"); + entityPickerModalLevel(1).should("not.exist"); + entityPickerModalLevel(2).get("[data-index]").should("have.length", 8); + assertDataPickerEntityNotSelected(2, "Accounts"); + assertDataPickerEntityNotSelected(2, "Analytic Events"); + assertDataPickerEntityNotSelected(2, "Feedback"); + assertDataPickerEntityNotSelected(2, "Invoices"); + assertDataPickerEntityNotSelected(2, "Orders"); + assertDataPickerEntityNotSelected(2, "People"); + assertDataPickerEntityNotSelected(2, "Products"); + assertDataPickerEntityNotSelected(2, "Reviews"); }); }); @@ -98,25 +113,19 @@ describe("scenarios > notebook > data source", () => { }); startNewQuestion(); - popover().within(() => { - cy.findByPlaceholderText("Search for some data…"); - cy.findAllByTestId("data-bucket-list-item") - .as("sources") - .should("have.length", 2); - cy.get("@sources") - .first() - .should("contain", "Models") - .and("have.attr", "aria-selected", "false"); - cy.get("@sources") - .last() - .should("contain", "Raw Data") - .and("have.attr", "aria-selected", "false"); + entityPickerModal().within(() => { + cy.findAllByRole("tab").should("have.length", 2); + entityPickerModalTab("Recents").should("not.exist"); + entityPickerModalTab("Models").and( + "have.attr", + "aria-selected", + "true", + ); + entityPickerModalTab("Tables").should("exist"); + entityPickerModalTab("Saved questions").should("not.exist"); }); }); - // There is a huge discrepancy between how we render this popover vs the one for models - // That's the reason this test is a bit vague. Will be reported as a separate issue - // and covered in a separate reproduction. it("should not show models if only saved questions exist", () => { createQuestion({ name: "GUI Question", @@ -125,11 +134,16 @@ describe("scenarios > notebook > data source", () => { }); startNewQuestion(); - popover().within(() => { - cy.get("[data-element-id=list-section-title]") - .should("have.length", 2) - .and("contain", "Saved Questions") - .and("not.contain", "Models"); + entityPickerModal().within(() => { + cy.findAllByRole("tab").should("have.length", 2); + entityPickerModalTab("Recents").should("not.exist"); + entityPickerModalTab("Models").should("not.exist"); + entityPickerModalTab("Tables").and( + "have.attr", + "aria-selected", + "true", + ); + entityPickerModalTab("Saved questions").should("exist"); }); }); }); @@ -144,16 +158,18 @@ describe("scenarios > notebook > data source", () => { openReviewsTable(); openNotebook(); cy.findByTestId("data-step-cell").should("have.text", "Reviews").click(); - popover().within(() => { - cy.findByTestId("source-database").should( - "have.text", - "Sample Database", - ); - cy.findByLabelText("Reviews").should( + entityPickerModal().within(() => { + entityPickerModalTab("Recents").should("exist"); + entityPickerModalTab("Tables").and( "have.attr", "aria-selected", "true", ); + // should not show databases step if there's only 1 database + entityPickerModalLevel(0).should("not.exist"); + // should not show schema step if there's only 1 schema + entityPickerModalLevel(1).should("not.exist"); + assertDataPickerEntitySelected(2, "Reviews"); }); }); @@ -161,16 +177,18 @@ describe("scenarios > notebook > data source", () => { visitQuestion(ORDERS_COUNT_QUESTION_ID); openNotebook(); cy.findByTestId("data-step-cell").should("have.text", "Orders").click(); - popover().within(() => { - cy.findByTestId("source-database").should( - "have.text", - "Sample Database", - ); - cy.findByLabelText("Orders").should( + entityPickerModal().within(() => { + entityPickerModalTab("Recents").should("exist"); + entityPickerModalTab("Tables").and( "have.attr", "aria-selected", "true", ); + // should not show databases step if there's only 1 database + entityPickerModalLevel(0).should("not.exist"); + // should not show schema step if there's only 1 schema + entityPickerModalLevel(1).should("not.exist"); + assertDataPickerEntitySelected(2, "Orders"); }); }); @@ -195,8 +213,9 @@ describe("scenarios > notebook > data source", () => { }); startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Recents").should("not.exist"); + entityPickerModalTab("Tables").click(); cy.findByText(dbName).click(); cy.findByText(schemaName).click(); cy.findByText(tableName).click(); @@ -206,9 +225,16 @@ describe("scenarios > notebook > data source", () => { openNotebook(); cy.findByTestId("data-step-cell").should("contain", tableName).click(); - popover().within(() => { - cy.findByTestId("source-database").should("have.text", dbName); - cy.findByTestId("source-schema").should("have.text", schemaName); + entityPickerModal().within(() => { + assertDataPickerEntitySelected(0, dbName); + assertDataPickerEntitySelected(1, schemaName); + assertDataPickerEntitySelected(2, tableName); + + entityPickerModalTab("Recents").click(); + cy.findByTestId("result-item") + .should("exist") + .and("contain.text", tableName) + .and("have.attr", "aria-selected", "true"); }); }, ); @@ -217,16 +243,17 @@ describe("scenarios > notebook > data source", () => { cy.visit(`/model/${ORDERS_MODEL_ID}/query`); cy.findByTestId("data-step-cell").should("have.text", "Orders").click(); - popover().within(() => { - cy.findByTestId("source-database").should( - "have.text", - "Sample Database", - ); - cy.findByLabelText("Orders").should( + entityPickerModal().within(() => { + entityPickerModalTab("Tables").should( "have.attr", "aria-selected", "true", ); + // should not show databases step if there's only 1 database + entityPickerModalLevel(0).should("not.exist"); + // should not show schema step if there's only 1 schema + entityPickerModalLevel(1).should("not.exist"); + assertDataPickerEntitySelected(2, "Orders"); }); }); }); @@ -252,26 +279,23 @@ describe("scenarios > notebook > data source", () => { .should("have.text", modelDetails.name) .click(); - cy.findByTestId("saved-entity-back-navigation").should( - "have.text", - "Models", - ); - - cy.findByTestId("saved-entity-collection-tree").within(() => { - cy.findByLabelText("Our analytics") - .should("have.attr", "aria-expanded", "false") - .and("have.attr", "aria-selected", "false"); - cy.findByLabelText("First collection") - .should("have.attr", "aria-expanded", "true") - .and("have.attr", "aria-selected", "false"); - cy.findByLabelText("Second collection") - .should("have.attr", "aria-expanded", "false") + entityPickerModal().within(() => { + entityPickerModalTab("Models").should( + "have.attr", + "aria-selected", + "true", + ); + assertDataPickerEntitySelected(0, "Our analytics"); + assertDataPickerEntitySelected(1, "First collection"); + assertDataPickerEntitySelected(2, "Second collection"); + assertDataPickerEntitySelected(3, checkNotNull(modelDetails.name)); + + entityPickerModalTab("Recents").click(); + cy.findByTestId("result-item") + .should("exist") + .and("contain.text", checkNotNull(modelDetails.name)) .and("have.attr", "aria-selected", "true"); }); - - cy.findByTestId("select-list") - .findByLabelText(checkNotNull(modelDetails.name)) - .should("have.attr", "aria-selected", "true"); }); it("moving the model to another collection should immediately be reflected in the data selector (metabase#39812-1)", () => { @@ -279,14 +303,43 @@ describe("scenarios > notebook > data source", () => { openNotebook(); openDataSelector(); - assertSourceCollection("Our analytics"); - assertDataSource("Orders Model"); + entityPickerModal().within(() => { + entityPickerModalTab("Models").should( + "have.attr", + "aria-selected", + "true", + ); + assertDataPickerEntitySelected(0, "Our analytics"); + assertDataPickerEntitySelected(1, "Orders Model"); + + entityPickerModalTab("Recents").click(); + cy.findByTestId("result-item") + .should("exist") + .and("contain.text", "Orders Model") + .and("have.attr", "aria-selected", "true"); + + cy.button("Close").click(); + }); moveToCollection("First collection"); openDataSelector(); - assertSourceCollection("First collection"); - assertDataSource("Orders Model"); + entityPickerModal().within(() => { + entityPickerModalTab("Models").should( + "have.attr", + "aria-selected", + "true", + ); + assertDataPickerEntitySelected(0, "Our analytics"); + assertDataPickerEntitySelected(1, "First collection"); + assertDataPickerEntitySelected(2, "Orders Model"); + + entityPickerModalTab("Recents").click(); + cy.findByTestId("result-item") + .should("exist") + .and("contain.text", "Orders Model") + .and("have.attr", "aria-selected", "true"); + }); }); it("moving the source question should immediately reflect in the data selector for the nested question that depends on it (metabase#39812-2)", () => { @@ -311,8 +364,24 @@ describe("scenarios > notebook > data source", () => { openNotebook(); openDataSelector(); - assertSourceCollection("Our analytics"); - assertDataSource(sourceQuestionName); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").should( + "have.attr", + "aria-selected", + "true", + ); + assertDataPickerEntitySelected(0, "Our analytics"); + assertDataPickerEntitySelected(1, sourceQuestionName); + + entityPickerModalTab("Recents").click(); + cy.findAllByTestId("result-item").should("have.length", 1); + cy.findByTestId("result-item") + .should("exist") + .and("contain.text", "Nested Question") + .and("not.have.attr", "aria-selected", "true"); + + cy.button("Close").click(); + }); cy.log("Move the source question to another collection"); visitQuestion(SOURCE_QUESTION_ID); @@ -324,8 +393,26 @@ describe("scenarios > notebook > data source", () => { openNotebook(); openDataSelector(); - assertSourceCollection("First collection"); - assertDataSource(sourceQuestionName); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").should( + "have.attr", + "aria-selected", + "true", + ); + assertDataPickerEntitySelected(0, "Our analytics"); + assertDataPickerEntitySelected(1, "First collection"); + assertDataPickerEntitySelected(2, sourceQuestionName); + + entityPickerModalTab("Recents").click(); + cy.findAllByTestId("result-item") + .contains(nestedQuestionDetails.name) + .parents("button") + .and("not.have.attr", "aria-selected", "true"); + cy.findAllByTestId("result-item") + .contains(sourceQuestionName) + .parents("button") + .and("have.attr", "aria-selected", "true"); + }); }); }); }); @@ -347,14 +434,10 @@ function openDataSelector() { cy.findByTestId("data-step-cell").click(); } -function assertItemSelected(item: string) { - cy.findByLabelText(item).should("have.attr", "aria-selected", "true"); -} - -function assertSourceCollection(collection: string) { - return assertItemSelected(collection); +function assertDataPickerEntitySelected(level: number, name: string) { + entityPickerModalItem(level, name).should("have.attr", "data-active", "true"); } -function assertDataSource(questionOrModel: string) { - return assertItemSelected(questionOrModel); +function assertDataPickerEntityNotSelected(level: number, name: string) { + entityPickerModalItem(level, name).should("not.have.attr", "data-active"); } diff --git a/e2e/test/scenarios/question/notebook-native-preview-sidebar.cy.spec.ts b/e2e/test/scenarios/question/notebook-native-preview-sidebar.cy.spec.ts index 9b459043de015..0e186b8b41617 100644 --- a/e2e/test/scenarios/question/notebook-native-preview-sidebar.cy.spec.ts +++ b/e2e/test/scenarios/question/notebook-native-preview-sidebar.cy.spec.ts @@ -25,6 +25,7 @@ import { expectGoodSnowplowEvent, expectGoodSnowplowEvents, expectNoBadSnowplowEvents, + entityPickerModal, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE; @@ -48,6 +49,7 @@ describe("scenarios > question > notebook > native query preview sidebar", () => "have.text", "Pick your starting data", ); + entityPickerModal().button("Close").click(); cy.findByTestId("native-query-preview-sidebar").within(() => { cy.findByText("SQL for this question").should("exist"); diff --git a/e2e/test/scenarios/question/notebook.cy.spec.js b/e2e/test/scenarios/question/notebook.cy.spec.js index 6f5bcca20871e..c4cd27e549909 100644 --- a/e2e/test/scenarios/question/notebook.cy.spec.js +++ b/e2e/test/scenarios/question/notebook.cy.spec.js @@ -23,6 +23,8 @@ import { visitQuestionAdhoc, visualize, createQuestion, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; import { createMetric } from "e2e/support/helpers/e2e-table-metadata-helpers"; @@ -520,7 +522,10 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => { openNotebook(); join(); - popover().findByText("Products model").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Models").click(); + cy.findByText("Products model").click(); + }); visualize(); }); @@ -576,19 +581,21 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => { }); startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Orders, Count").click(); + + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("Orders, Count").click(); + }); + visualize(); }, ); it('should not show "median" aggregation option for databases that do not support "percentile-aggregations" driver feature', () => { startNewQuestion(); - popover().within(() => { - cy.contains("Raw Data").click(); - cy.contains("Orders").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Orders").click(); }); getNotebookStep("summarize") @@ -691,10 +698,12 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => { 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(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Orders").click(); + }); + cy.get("@dataStep").icon("play").should("be.visible"); getNotebookStep("filter").icon("play").should("not.be.visible"); getNotebookStep("summarize").icon("play").should("not.be.visible"); diff --git a/e2e/test/scenarios/question/offset.cy.spec.ts b/e2e/test/scenarios/question/offset.cy.spec.ts index 81976a098318c..21fa8edc9c196 100644 --- a/e2e/test/scenarios/question/offset.cy.spec.ts +++ b/e2e/test/scenarios/question/offset.cy.spec.ts @@ -3,6 +3,8 @@ import { createQuestion, echartsContainer, enterCustomColumnDetails, + entityPickerModal, + entityPickerModalTab, getNotebookStep, modal, openNotebook, @@ -185,8 +187,8 @@ describe("scenarios > question > offset", () => { const breakoutName = "Created At"; startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); addCustomAggregation({ @@ -229,8 +231,8 @@ describe("scenarios > question > offset", () => { ]; startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); addCustomAggregation({ diff --git a/e2e/test/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js b/e2e/test/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js index 2fcea3fdcd411..df97d13e10388 100644 --- a/e2e/test/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js @@ -8,6 +8,8 @@ import { visitDashboard, openColumnOptions, modal, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; import { setAdHocFilter } from "../../native-filters/helpers/e2e-date-filter-helpers"; @@ -147,8 +149,10 @@ describe("issue 17514", () => { // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Join data").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Products").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText("Products").click(); + }); cy.button("Visualize").click(); diff --git a/e2e/test/scenarios/question/reproductions/19341-disabled-nested-queries.cy.spec.js b/e2e/test/scenarios/question/reproductions/19341-disabled-nested-queries.cy.spec.js index 5cd73eff3444f..d1dd6a5a01439 100644 --- a/e2e/test/scenarios/question/reproductions/19341-disabled-nested-queries.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/19341-disabled-nested-queries.cy.spec.js @@ -1,7 +1,8 @@ import { - restore, + entityPickerModal, + entityPickerModalTab, mockSessionProperty, - popover, + restore, startNewQuestion, } from "e2e/support/helpers"; @@ -24,22 +25,16 @@ describe("issue 19341", () => { it("should correctly disable nested queries (metabase#19341)", () => { // Test "Saved Questions" table is hidden in QB data selector startNewQuestion(); - popover().within(() => { - // Wait until picker init - // When working as expected, the test environment only has "Sample Database" DB - // So it should automatically select it as a database - // When "Orders" table name appears, it means the picker has selected the sample database - cy.findByText("Loading...").should("not.exist"); - cy.findByText("Orders"); - - cy.findByText("Sample Database").click(); // go back to DB list - cy.findByText("Saved Questions").should("not.exist"); + entityPickerModal().within(() => { + cy.findByTestId("loading-spinner").should("not.exist"); + cy.findByText("Orders").should("exist"); + cy.findAllByRole("tab").should("not.exist"); // Ensure the search doesn't list saved questions - cy.findByPlaceholderText("Search for a table…").type("Ord"); - cy.findByText("Loading...").should("not.exist"); + cy.findByPlaceholderText("Search…").type("Ord"); + cy.findByTestId("loading-spinner").should("not.exist"); - cy.findAllByTestId("search-result-item").then($result => { + cy.findAllByTestId("result-item").then($result => { const searchResults = $result.toArray(); const modelTypes = new Set( searchResults.map(k => k.getAttribute("data-model-type")), @@ -49,17 +44,12 @@ describe("issue 19341", () => { expect(modelTypes).to.include("table"); }); - cy.icon("close").click(); - - cy.findByText("Sample Database").click(); + entityPickerModalTab("Tables").click(); cy.findByText("Orders").click(); }); cy.icon("join_left_outer").click(); - popover().within(() => { - cy.findByText("Sample Database").click(); // go back to DB list - cy.findByText("Saved Questions").should("not.exist"); - }); + entityPickerModal().findAllByRole("tab").should("not.exist"); // Test "Explore results" button is hidden for native questions cy.visit("/collection/root"); diff --git a/e2e/test/scenarios/question/reproductions/19742-data-picker-closes-after-hiding-table.cy.spec.js b/e2e/test/scenarios/question/reproductions/19742-data-picker-closes-after-hiding-table.cy.spec.js index cf04360592c7c..2b01e63775fb8 100644 --- a/e2e/test/scenarios/question/reproductions/19742-data-picker-closes-after-hiding-table.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/19742-data-picker-closes-after-hiding-table.cy.spec.js @@ -1,4 +1,10 @@ -import { restore, popover, openNavigationSidebar } from "e2e/support/helpers"; +import { + restore, + popover, + openNavigationSidebar, + entityPickerModal, + entityPickerModalTab, +} from "e2e/support/helpers"; describe("issue 19742", () => { beforeEach(() => { @@ -12,11 +18,12 @@ describe("issue 19742", () => { cy.visit("/"); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("New").click(); - selectFromDropdown("Question"); - selectFromDropdown("Raw Data"); - popover().within(() => { + popover().findByText("Question").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("Orders").should("exist"); + cy.button("Close").click(); }); openNavigationSidebar(); @@ -31,14 +38,15 @@ describe("issue 19742", () => { // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("New").click(); - selectFromDropdown("Question"); - selectFromDropdown("Raw Data"); + popover().findByText("Question").click(); + + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); - popover().within(() => { - cy.findByText("Products"); - cy.findByText("Reviews"); - cy.findByText("People"); cy.findByText("Orders").should("not.exist"); + cy.findByText("Products").should("exist"); + cy.findByText("Reviews").should("exist"); + cy.findByText("People").should("exist"); }); }); }); diff --git a/e2e/test/scenarios/question/reproductions/20627-nested-long-names-wrong-aliases.cy.spec.js b/e2e/test/scenarios/question/reproductions/20627-nested-long-names-wrong-aliases.cy.spec.js index 19922ecba7676..b89f3ffe703ea 100644 --- a/e2e/test/scenarios/question/reproductions/20627-nested-long-names-wrong-aliases.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/20627-nested-long-names-wrong-aliases.cy.spec.js @@ -5,6 +5,8 @@ import { popover, enterCustomColumnDetails, visualize, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; const { ORDERS, PRODUCTS_ID } = SAMPLE_DATABASE; @@ -26,8 +28,11 @@ describe("issue 20627", () => { // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Join data").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText(newTableName).click(); + + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText(newTableName).click(); + }); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Summarize").click(); diff --git a/e2e/test/scenarios/question/reproductions/22285-schema-picker.cy.spec.js b/e2e/test/scenarios/question/reproductions/22285-schema-picker.cy.spec.js deleted file mode 100644 index 0384c57364efd..0000000000000 --- a/e2e/test/scenarios/question/reproductions/22285-schema-picker.cy.spec.js +++ /dev/null @@ -1,35 +0,0 @@ -import { restore, startNewQuestion, popover } from "e2e/support/helpers"; - -describe("issue 22285", () => { - beforeEach(() => { - restore(); - cy.signInAsAdmin(); - - cy.intercept("GET", "/api/database").as("fetchDatabases"); - - cy.intercept("GET", "/api/database/*/schemas", { - body: ["PUBLIC", "FAKE SCHEMA"], - }); - }); - - it("should not clean DB schemas list in the data selector (metabase#22285)", () => { - startNewQuestion(); - cy.wait("@fetchDatabases"); - - popover().within(() => { - cy.findByText("Raw Data").click(); - cy.findByText("Sample Database").click(); - - cy.findByText(/Fake Schema/i); - cy.findByText(/Public/i).click(); - cy.findByText("Orders"); - - // go back to database picker - cy.icon("chevronleft").click(); - - cy.findByText("Sample Database").click(); - cy.findByText(/Fake Schema/i); - cy.findByText(/Public/i); - }); - }); -}); diff --git a/e2e/test/scenarios/question/reproductions/30610-stale-results-metadata.cy.spec.js b/e2e/test/scenarios/question/reproductions/30610-stale-results-metadata.cy.spec.js index eaa3d7ff093c4..4df4375459520 100644 --- a/e2e/test/scenarios/question/reproductions/30610-stale-results-metadata.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/30610-stale-results-metadata.cy.spec.js @@ -1,11 +1,14 @@ import { ORDERS_QUESTION_ID } from "e2e/support/cypress_sample_instance_data"; import { + entityPickerModal, + entityPickerModalTab, openNotebook, openOrdersTable, popover, queryBuilderHeader, restore, saveQuestion, + startNewQuestion, visitQuestion, visualize, } from "e2e/support/helpers"; @@ -48,10 +51,9 @@ function removeSourceColumns() { } function createAdHocQuestion(questionName) { - cy.findByTestId("app-bar").findByText("New").click(); - popover().within(() => { - cy.findByText("Question").click(); - cy.findByText("Saved Questions").click(); + startNewQuestion(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); cy.findByText(questionName).click(); }); cy.findByTestId("fields-picker").click(); diff --git a/e2e/test/scenarios/question/reproductions/38354-changing-source-database.cy.spec.js b/e2e/test/scenarios/question/reproductions/38354-changing-source-database.cy.spec.js index 23698675b1364..9146b070feda9 100644 --- a/e2e/test/scenarios/question/reproductions/38354-changing-source-database.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/38354-changing-source-database.cy.spec.js @@ -1,8 +1,8 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { + entityPickerModal, getNotebookStep, openNotebook, - popover, restore, visualize, } from "e2e/support/helpers"; @@ -27,8 +27,7 @@ describe("issue 38354", { tags: "@external" }, () => { it("should be possible to change source database (metabase#38354)", () => { openNotebook(); getNotebookStep("data").findByTestId("data-step-cell").click(); - popover().within(() => { - cy.icon("chevronleft").click(); + entityPickerModal().within(() => { cy.findByText("QA Postgres12").click(); cy.findByText("Orders").click(); }); diff --git a/e2e/test/scenarios/question/reproductions/9027-new-questions-not-in-saved-questions-immediately.cy.spec.js b/e2e/test/scenarios/question/reproductions/9027-new-questions-not-in-saved-questions-immediately.cy.spec.js index 32b7d2ee48bde..aa5961b3348a8 100644 --- a/e2e/test/scenarios/question/reproductions/9027-new-questions-not-in-saved-questions-immediately.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/9027-new-questions-not-in-saved-questions-immediately.cy.spec.js @@ -5,6 +5,8 @@ import { startNewQuestion, openNavigationSidebar, navigationSidebar, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; const QUESTION_NAME = "Foo"; @@ -15,12 +17,11 @@ describe("issue 9027", () => { cy.signInAsAdmin(); startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - - // Wait for the existing questions to load - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Orders"); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("Orders").should("exist"); + cy.button("Close").click(); + }); openNativeEditor({ fromCurrentPage: true }); @@ -43,9 +44,11 @@ describe("issue 9027", () => { function goToSavedQuestionPickerAndAssertQuestion(questionName, exists = true) { startNewQuestion(); - cy.findByText("Saved Questions").click(); - - cy.findByText(questionName).should(exists ? "exist" : "not.exist"); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText(questionName).should(exists ? "exist" : "not.exist"); + cy.button("Close").click(); + }); } function saveQuestion(name) { diff --git a/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js b/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js index c34efd2c09035..cece2cd0ea842 100644 --- a/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js +++ b/e2e/test/scenarios/sharing/downloads/downloads.cy.spec.js @@ -25,6 +25,8 @@ import { queryBuilderMain, editDashboard, setFilter, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE; @@ -63,10 +65,10 @@ describe("scenarios > question > download", () => { testCases.forEach(fileType => { it(`downloads ${fileType} file`, () => { startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Orders, Count").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.findByText("Orders, Count").click(); + }); visualize(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage diff --git a/e2e/test/scenarios/sharing/public-sharing-embed-button-behavior.cy.spec.js b/e2e/test/scenarios/sharing/public-sharing-embed-button-behavior.cy.spec.js index ebf4483de53b6..2bb18d886c94e 100644 --- a/e2e/test/scenarios/sharing/public-sharing-embed-button-behavior.cy.spec.js +++ b/e2e/test/scenarios/sharing/public-sharing-embed-button-behavior.cy.spec.js @@ -4,6 +4,8 @@ import { describeEE, describeWithSnowplow, enableTracking, + entityPickerModal, + entityPickerModalTab, expectGoodSnowplowEvent, expectNoBadSnowplowEvents, getEmbedModalSharingPane, @@ -258,8 +260,8 @@ describe("#39152 sharing an unsaved question", () => { it("should ask the user to save the question before creating a public link", () => { startNewQuestion(); - popover().within(() => { - cy.findByText("Raw Data").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); cy.findByText("People").click(); }); visualize(); diff --git a/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js index 073a70d058a73..9daf970c38305 100644 --- a/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js +++ b/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js @@ -17,6 +17,8 @@ import { chartPathWithFillColor, echartsContainer, cartesianChartCircleWithColor, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID, PEOPLE, PEOPLE_ID } = @@ -255,10 +257,10 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => { }); // Build a new question off that grouping by City startNewQuestion(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Saved Questions").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("CA People").click(); + entityPickerModal().within(() => { + entityPickerModalTab("Saved questions").click(); + cy.contains("CA People").click(); + }); addSummaryField({ metric: "Count of rows" }); diff --git a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js index ac5fbbafc5a99..9213cecfec7ec 100644 --- a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js +++ b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js @@ -20,6 +20,8 @@ import { moveDnDKitElement, selectFilterOperator, expressionEditorWidget, + entityPickerModal, + entityPickerModalTab, } from "e2e/support/helpers"; describe("scenarios > visualizations > table", () => { @@ -31,7 +33,10 @@ describe("scenarios > visualizations > table", () => { function joinTable(table) { cy.findByText("Join data").click(); - popover().findByText(table).click(); + entityPickerModal().within(() => { + entityPickerModalTab("Tables").click(); + cy.findByText(table).click(); + }); } function selectFromDropdown(option, clickOpts) { diff --git a/frontend/src/metabase-lib/join.ts b/frontend/src/metabase-lib/join.ts index bdbe546c8fc6a..1a0921fe9d32a 100644 --- a/frontend/src/metabase-lib/join.ts +++ b/frontend/src/metabase-lib/join.ts @@ -1,4 +1,10 @@ import * as ML from "cljs/metabase.lib.js"; +import type { + CardId, + ConcreteTableId, + DatabaseId, + VirtualTableId, +} from "metabase-types/api"; import { expressionParts } from "./expression"; import { isColumnMetadata } from "./internal"; @@ -237,13 +243,22 @@ export function joinedThing(query: Query, join: Join): Joinable { return ML.joined_thing(query, join); } -export type PickerInfo = { - databaseId: number; - tableId: string; - cardId?: number; - isModel?: boolean; +type CardPickerInfo = { + databaseId: DatabaseId; + tableId: VirtualTableId; + cardId: CardId; + isModel: boolean; +}; + +type TablePickerInfo = { + databaseId: DatabaseId; + tableId: ConcreteTableId; + cardId?: never; + isModel?: never; }; +export type PickerInfo = TablePickerInfo | CardPickerInfo; + /** * Returns `null` when the joined table/card isn't available, e.g. due to sandboxing. */ diff --git a/frontend/src/metabase-lib/order_by.unit.spec.ts b/frontend/src/metabase-lib/order_by.unit.spec.ts index 0bbe0393c6c7d..9358808ff4936 100644 --- a/frontend/src/metabase-lib/order_by.unit.spec.ts +++ b/frontend/src/metabase-lib/order_by.unit.spec.ts @@ -36,6 +36,7 @@ describe("order by", () => { displayName: "Orders", longDisplayName: "Orders", isSourceTable: true, + schema: "1:PUBLIC", }, }), ); @@ -60,6 +61,7 @@ describe("order by", () => { displayName: "Products", longDisplayName: "Products", isSourceTable: false, + schema: "1:PUBLIC", }, }), ); diff --git a/frontend/src/metabase-lib/types.ts b/frontend/src/metabase-lib/types.ts index 387abbe9a3ae7..5869f21115bb5 100644 --- a/frontend/src/metabase-lib/types.ts +++ b/frontend/src/metabase-lib/types.ts @@ -6,6 +6,7 @@ import type { FieldValuesType, RowValue, TableId, + SchemaId, } from "metabase-types/api"; import type { @@ -134,6 +135,8 @@ export type TableDisplayInfo = { isSourceTable: boolean; isFromJoin: boolean; isImplicitlyJoinable: boolean; + schema: SchemaId; + isModel?: boolean; }; export type CardDisplayInfo = TableDisplayInfo; diff --git a/frontend/src/metabase-lib/v1/Question.ts b/frontend/src/metabase-lib/v1/Question.ts index 913284408d1c8..f342974775cd1 100644 --- a/frontend/src/metabase-lib/v1/Question.ts +++ b/frontend/src/metabase-lib/v1/Question.ts @@ -496,11 +496,11 @@ class Question { return this.setCard(assoc(this.card(), "name", name)); } - collectionId(): number | null | undefined { + collectionId(): CollectionId | null | undefined { return this._card && this._card.collection_id; } - setCollectionId(collectionId: number | null | undefined) { + setCollectionId(collectionId: CollectionId | null | undefined) { return this.setCard(assoc(this.card(), "collection_id", collectionId)); } diff --git a/frontend/src/metabase-types/api/schema.ts b/frontend/src/metabase-types/api/schema.ts index 5401ad725f9c7..279ef70d0bc2a 100644 --- a/frontend/src/metabase-types/api/schema.ts +++ b/frontend/src/metabase-types/api/schema.ts @@ -8,7 +8,14 @@ import type { Field, FieldDimension, FieldId } from "./field"; import type { Metric, MetricId } from "./metric"; import type { Segment, SegmentId } from "./segment"; import type { NativeQuerySnippet } from "./snippets"; -import type { ForeignKey, Schema, SchemaId, Table, TableId } from "./table"; +import type { + ForeignKey, + Schema, + SchemaId, + SchemaName, + Table, + TableId, +} from "./table"; import type { Timeline, TimelineEventId } from "./timeline"; import type { User } from "./user"; @@ -41,7 +48,7 @@ export interface NormalizedTable segments?: SegmentId[]; metrics?: MetricId[]; schema?: SchemaId; - schema_name?: string; + schema_name?: SchemaName; } export interface NormalizedForeignKey diff --git a/frontend/src/metabase-types/api/table.ts b/frontend/src/metabase-types/api/table.ts index a45e226f7e200..1cb296277a52c 100644 --- a/frontend/src/metabase-types/api/table.ts +++ b/frontend/src/metabase-types/api/table.ts @@ -30,7 +30,7 @@ export type Table = { db_id: DatabaseId; db?: Database; - schema: string; + schema: SchemaName; fks?: ForeignKey[]; fields?: Field[]; diff --git a/frontend/src/metabase/api/database.ts b/frontend/src/metabase/api/database.ts index 1981e7f8ff06c..785bb8a290272 100644 --- a/frontend/src/metabase/api/database.ts +++ b/frontend/src/metabase/api/database.ts @@ -13,6 +13,7 @@ import type { ListDatabaseSchemaTablesRequest, ListDatabaseSchemasRequest, ListVirtualDatabaseTablesRequest, + SchemaName, } from "metabase-types/api"; import { Api } from "./api"; @@ -54,7 +55,10 @@ export const databaseApi = Api.injectEndpoints({ }), providesTags: database => (database ? provideDatabaseTags(database) : []), }), - listDatabaseSchemas: builder.query({ + listDatabaseSchemas: builder.query< + SchemaName[], + ListDatabaseSchemasRequest + >({ query: ({ id, ...body }) => ({ method: "GET", url: `/api/database/${id}/schemas`, @@ -65,7 +69,7 @@ export const databaseApi = Api.injectEndpoints({ ...schemas.map(schema => idTag("schema", schema)), ], }), - listSyncableDatabaseSchemas: builder.query({ + listSyncableDatabaseSchemas: builder.query({ query: id => ({ method: "GET", url: `/api/database/${id}/syncable_schemas`, diff --git a/frontend/src/metabase/api/table.ts b/frontend/src/metabase/api/table.ts index f2aeee336b3c3..49cb827bb9e64 100644 --- a/frontend/src/metabase/api/table.ts +++ b/frontend/src/metabase/api/table.ts @@ -4,6 +4,7 @@ import type { GetTableRequest, Table, TableId, + TableListQuery, UpdateTableFieldsOrderRequest, UpdateTableListRequest, UpdateTableRequest, @@ -21,10 +22,11 @@ import { export const tableApi = Api.injectEndpoints({ endpoints: builder => ({ - listTables: builder.query({ - query: () => ({ + listTables: builder.query({ + query: body => ({ method: "GET", url: "/api/table", + body, }), providesTags: (tables = []) => provideTableListTags(tables), }), diff --git a/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemList.tsx b/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemList.tsx index 39977ca8f5596..69918960af56c 100644 --- a/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemList.tsx +++ b/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemList.tsx @@ -10,13 +10,16 @@ export const CollectionItemList = ({ isFolder, isCurrentLevel, shouldDisableItem, + shouldShowItem, }: CollectionItemListProps) => { const { data: collectionItems, error, isLoading, } = useListCollectionItemsQuery<{ - data: { data: CollectionPickerItem[] }; + data: { + data: CollectionPickerItem[]; + }; error: any; isLoading: boolean; }>(query ? query : skipToken); @@ -31,6 +34,7 @@ export const CollectionItemList = ({ isFolder={isFolder} isCurrentLevel={isCurrentLevel} shouldDisableItem={shouldDisableItem} + shouldShowItem={shouldShowItem} /> ); }; diff --git a/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemPickerResolver.tsx b/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemPickerResolver.tsx index 718083c5a6f4e..1c39ed080f59a 100644 --- a/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemPickerResolver.tsx +++ b/frontend/src/metabase/common/components/CollectionPicker/components/CollectionItemPickerResolver.tsx @@ -14,6 +14,7 @@ export const CollectionItemPickerResolver = ({ isFolder, isCurrentLevel, shouldDisableItem, + shouldShowItem, }: CollectionItemListProps) => { if (!query) { return ( @@ -24,6 +25,7 @@ export const CollectionItemPickerResolver = ({ isFolder={isFolder} isCurrentLevel={isCurrentLevel} shouldDisableItem={shouldDisableItem} + shouldShowItem={shouldShowItem} /> ); } @@ -36,6 +38,7 @@ export const CollectionItemPickerResolver = ({ isFolder={isFolder} isCurrentLevel={isCurrentLevel} shouldDisableItem={shouldDisableItem} + shouldShowItem={shouldShowItem} options={options} /> ); @@ -49,6 +52,7 @@ export const CollectionItemPickerResolver = ({ isFolder={isFolder} isCurrentLevel={isCurrentLevel} shouldDisableItem={shouldDisableItem} + shouldShowItem={shouldShowItem} options={options} /> ); diff --git a/frontend/src/metabase/common/components/CollectionPicker/components/CollectionPickerModal.tsx b/frontend/src/metabase/common/components/CollectionPicker/components/CollectionPickerModal.tsx index 60ac1998a2327..aa42fe1ff0115 100644 --- a/frontend/src/metabase/common/components/CollectionPicker/components/CollectionPickerModal.tsx +++ b/frontend/src/metabase/common/components/CollectionPicker/components/CollectionPickerModal.tsx @@ -76,17 +76,19 @@ export const CollectionPickerModal = ({ } }; - const modalActions = [ - , - ]; + const modalActions = options.allowCreateNew + ? [ + , + ] + : []; const tabs: [EntityTab] = [ { diff --git a/frontend/src/metabase/common/components/CollectionPicker/components/PersonalCollectionItemList.tsx b/frontend/src/metabase/common/components/CollectionPicker/components/PersonalCollectionItemList.tsx index d9063593fe1a5..b1c3198649bb8 100644 --- a/frontend/src/metabase/common/components/CollectionPicker/components/PersonalCollectionItemList.tsx +++ b/frontend/src/metabase/common/components/CollectionPicker/components/PersonalCollectionItemList.tsx @@ -12,6 +12,7 @@ export const PersonalCollectionsItemList = ({ isFolder, isCurrentLevel, shouldDisableItem, + shouldShowItem, }: CollectionItemListProps) => { const { data: collections, @@ -36,6 +37,7 @@ export const PersonalCollectionsItemList = ({ isFolder={isFolder} isCurrentLevel={isCurrentLevel} shouldDisableItem={shouldDisableItem} + shouldShowItem={shouldShowItem} /> ); }; diff --git a/frontend/src/metabase/common/components/CollectionPicker/components/RootItemList.tsx b/frontend/src/metabase/common/components/CollectionPicker/components/RootItemList.tsx index fa49066790ef3..2318b4caf87a7 100644 --- a/frontend/src/metabase/common/components/CollectionPicker/components/RootItemList.tsx +++ b/frontend/src/metabase/common/components/CollectionPicker/components/RootItemList.tsx @@ -36,6 +36,7 @@ export const RootItemList = ({ isFolder, isCurrentLevel, shouldDisableItem, + shouldShowItem, }: CollectionItemListProps) => { const isAdmin = useSelector(getUserIsAdmin); const currentUser = useSelector(getUser); @@ -135,6 +136,7 @@ export const RootItemList = ({ isFolder={isFolder} isCurrentLevel={isCurrentLevel} shouldDisableItem={shouldDisableItem} + shouldShowItem={shouldShowItem} /> ); }; diff --git a/frontend/src/metabase/common/components/CollectionPicker/types.ts b/frontend/src/metabase/common/components/CollectionPicker/types.ts index e2bc286fbe388..617e9c98b33f5 100644 --- a/frontend/src/metabase/common/components/CollectionPicker/types.ts +++ b/frontend/src/metabase/common/components/CollectionPicker/types.ts @@ -47,6 +47,7 @@ export type CollectionPickerValueItem = Omit & { }; export type CollectionPickerOptions = EntityPickerModalOptions & { + allowCreateNew?: boolean; showPersonalCollections?: boolean; showRootCollection?: boolean; namespace?: "snippets"; diff --git a/frontend/src/metabase/common/components/DashboardPicker/components/DashboardPicker.tsx b/frontend/src/metabase/common/components/DashboardPicker/components/DashboardPicker.tsx index 3e4b0f3849384..c22611f12a0f0 100644 --- a/frontend/src/metabase/common/components/DashboardPicker/components/DashboardPicker.tsx +++ b/frontend/src/metabase/common/components/DashboardPicker/components/DashboardPicker.tsx @@ -32,6 +32,7 @@ export const defaultOptions: DashboardPickerOptions = { showRootCollection: true, allowCreateNew: true, }; + interface DashboardPickerProps { onItemSelect: (item: DashboardPickerItem) => void; initialValue?: Pick; diff --git a/frontend/src/metabase/common/components/DashboardPicker/types.ts b/frontend/src/metabase/common/components/DashboardPicker/types.ts index 17fd690055b64..493892fbbf200 100644 --- a/frontend/src/metabase/common/components/DashboardPicker/types.ts +++ b/frontend/src/metabase/common/components/DashboardPicker/types.ts @@ -34,6 +34,7 @@ export type DashboardPickerInitialValueItem = { export type DashboardPickerItem = CollectionPickerItem; export type DashboardPickerOptions = EntityPickerModalOptions & { + allowCreateNew?: boolean; showPersonalCollections?: boolean; showRootCollection?: boolean; }; diff --git a/frontend/src/metabase/common/components/DataPicker/components/DataPickerModal.tsx b/frontend/src/metabase/common/components/DataPicker/components/DataPickerModal.tsx new file mode 100644 index 0000000000000..4df5702ea3800 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/components/DataPickerModal.tsx @@ -0,0 +1,163 @@ +import { useCallback, useMemo } from "react"; +import { t } from "ttag"; + +import { useSetting } from "metabase/common/hooks"; +import { getQuestionVirtualTableId } from "metabase-lib/v1/metadata/utils/saved-questions"; +import type { + CollectionItemModel, + DatabaseId, + TableId, +} from "metabase-types/api"; + +import type { EntityTab } from "../../EntityPicker"; +import { EntityPickerModal, defaultOptions } from "../../EntityPicker"; +import type { QuestionPickerItem } from "../../QuestionPicker"; +import { QuestionPicker } from "../../QuestionPicker"; +import { useAvailableData } from "../hooks"; +import type { + DataPickerModalOptions, + DataPickerValue, + NotebookDataPickerValueItem, +} from "../types"; +import { + createShouldShowItem, + isModelItem, + isQuestionItem, + isTableItem, + isValidValueItem, +} from "../utils"; + +import { TablePicker } from "./TablePicker"; + +interface Props { + /** + * Limit selection to a particular database + */ + databaseId?: DatabaseId; + title: string; + value: DataPickerValue | undefined; + onChange: (value: TableId) => void; + onClose: () => void; +} + +const MODEL_PICKER_MODELS: CollectionItemModel[] = ["dataset"]; + +const QUESTION_PICKER_MODELS: CollectionItemModel[] = ["card"]; + +const options: DataPickerModalOptions = { + ...defaultOptions, + hasConfirmButtons: false, + showPersonalCollections: true, + showRootCollection: true, + hasRecents: true, +}; + +export const DataPickerModal = ({ + databaseId, + title, + value, + onChange, + onClose, +}: Props) => { + const hasNestedQueriesEnabled = useSetting("enable-nested-queries"); + const { hasModels, hasQuestions } = useAvailableData({ databaseId }); + + const shouldShowItem = useMemo(() => { + return createShouldShowItem(databaseId); + }, [databaseId]); + + const searchParams = useMemo(() => { + return databaseId ? { table_db_id: databaseId } : undefined; + }, [databaseId]); + + const handleChange = useCallback( + (item: NotebookDataPickerValueItem) => { + if (!isValidValueItem(item.model)) { + return; + } + + const id = + item.model === "table" ? item.id : getQuestionVirtualTableId(item.id); + onChange(id); + onClose(); + }, + [onChange, onClose], + ); + + const handleCardChange = useCallback( + (item: QuestionPickerItem) => { + if (!isValidValueItem(item.model)) { + return; + } + + onChange(getQuestionVirtualTableId(item.id)); + onClose(); + }, + [onChange, onClose], + ); + + const tabs: EntityTab[] = [ + hasModels && hasNestedQueriesEnabled + ? { + displayName: t`Models`, + model: "dataset", + icon: "model", + element: ( + + ), + } + : undefined, + { + displayName: t`Tables`, + model: "table", + icon: "table", + element: ( + + ), + }, + hasQuestions && hasNestedQueriesEnabled + ? { + displayName: t`Saved questions`, + model: "card", + icon: "folder", + element: ( + + ), + } + : undefined, + ].filter( + (tab): tab is EntityTab => + tab != null, + ); + + return ( + + ); +}; diff --git a/frontend/src/metabase/common/components/DataPicker/components/DatabaseList.tsx b/frontend/src/metabase/common/components/DataPicker/components/DatabaseList.tsx new file mode 100644 index 0000000000000..2b8406db63537 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/components/DatabaseList.tsx @@ -0,0 +1,59 @@ +import { useMemo } from "react"; + +import type { Database } from "metabase-types/api"; + +import { ItemList, ListBox } from "../../EntityPicker"; +import { useAutoSelectOnlyItem } from "../hooks"; +import type { NotebookDataPickerFolderItem } from "../types"; + +interface Props { + databases: Database[] | undefined; + error: unknown; + isCurrentLevel: boolean; + isLoading: boolean; + selectedItem: NotebookDataPickerFolderItem | null; + onClick: (item: NotebookDataPickerFolderItem) => void; +} + +const isFolder = () => true; + +export const DatabaseList = ({ + databases, + error, + isCurrentLevel, + isLoading, + selectedItem, + onClick, +}: Props) => { + const items: NotebookDataPickerFolderItem[] | undefined = useMemo(() => { + return databases?.map(database => ({ + id: database.id, + model: "database", + name: database.name, + })); + }, [databases]); + + const hasOnly1Item = useAutoSelectOnlyItem({ + disabled: Boolean(selectedItem), + items, + onChange: onClick, + }); + + if (!isLoading && !error && hasOnly1Item) { + return null; + } + + return ( + + + + ); +}; diff --git a/frontend/src/metabase/common/components/DataPicker/components/SchemaList.tsx b/frontend/src/metabase/common/components/DataPicker/components/SchemaList.tsx new file mode 100644 index 0000000000000..a03acaf5af06e --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/components/SchemaList.tsx @@ -0,0 +1,60 @@ +import { useMemo } from "react"; + +import type { SchemaName } from "metabase-types/api"; + +import { ItemList, ListBox } from "../../EntityPicker"; +import { useAutoSelectOnlyItem } from "../hooks"; +import type { NotebookDataPickerFolderItem } from "../types"; +import { getSchemaDisplayName } from "../utils"; + +interface Props { + error: unknown; + isCurrentLevel: boolean; + isLoading: boolean; + schemas: SchemaName[] | undefined; + selectedItem: NotebookDataPickerFolderItem | null; + onClick: (item: NotebookDataPickerFolderItem) => void; +} + +const isFolder = () => true; + +export const SchemaList = ({ + error, + isCurrentLevel, + isLoading, + schemas, + selectedItem, + onClick, +}: Props) => { + const items: NotebookDataPickerFolderItem[] | undefined = useMemo(() => { + return schemas?.map(schema => ({ + id: schema, + model: "schema", + name: getSchemaDisplayName(schema), + })); + }, [schemas]); + + const hasOnly1Item = useAutoSelectOnlyItem({ + disabled: Boolean(selectedItem), + items, + onChange: onClick, + }); + + if (!isLoading && !error && hasOnly1Item) { + return null; + } + + return ( + + + + ); +}; diff --git a/frontend/src/metabase/common/components/DataPicker/components/TableList.tsx b/frontend/src/metabase/common/components/DataPicker/components/TableList.tsx new file mode 100644 index 0000000000000..c7c7d5b884780 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/components/TableList.tsx @@ -0,0 +1,48 @@ +import { useMemo } from "react"; + +import type { Table } from "metabase-types/api"; + +import { ItemList, ListBox } from "../../EntityPicker"; +import type { NotebookDataPickerValueItem } from "../types"; + +interface Props { + error: unknown; + isLoading: boolean; + isCurrentLevel: boolean; + selectedItem: NotebookDataPickerValueItem | null; + tables: Table[] | undefined; + onClick: (item: NotebookDataPickerValueItem) => void; +} + +const isFolder = () => false; + +export const TableList = ({ + error, + isLoading, + isCurrentLevel, + selectedItem, + tables, + onClick, +}: Props) => { + const items: NotebookDataPickerValueItem[] | undefined = useMemo(() => { + return tables?.map(table => ({ + id: table.id, + model: "table", + name: table.display_name, + })); + }, [tables]); + + return ( + + + + ); +}; diff --git a/frontend/src/metabase/common/components/DataPicker/components/TablePicker.tsx b/frontend/src/metabase/common/components/DataPicker/components/TablePicker.tsx new file mode 100644 index 0000000000000..a4a98885761dc --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/components/TablePicker.tsx @@ -0,0 +1,153 @@ +import { useCallback, useMemo, useState } from "react"; + +import { + skipToken, + useListDatabaseSchemaTablesQuery, + useListDatabaseSchemasQuery, + useListDatabasesQuery, +} from "metabase/api"; +import { isNotNull } from "metabase/lib/types"; +import { Flex } from "metabase/ui"; +import type { DatabaseId, SchemaName, TableId } from "metabase-types/api"; + +import { AutoScrollBox } from "../../EntityPicker"; +import type { + NotebookDataPickerFolderItem, + NotebookDataPickerValueItem, + TablePickerValue, +} from "../types"; +import { generateKey, getDbItem, getSchemaItem, getTableItem } from "../utils"; + +import { DatabaseList } from "./DatabaseList"; +import { SchemaList } from "./SchemaList"; +import { TableList } from "./TableList"; + +interface Props { + /** + * Limit selection to a particular database + */ + databaseId?: DatabaseId; + value: TablePickerValue | undefined; + onChange: (value: NotebookDataPickerValueItem) => void; +} + +export const TablePicker = ({ databaseId, value, onChange }: Props) => { + const [dbId, setDbId] = useState( + databaseId ?? value?.db_id, + ); + const [schemaName, setSchemaName] = useState( + value?.schema, + ); + const [tableId, setTableId] = useState(value?.id); + + const { + data: databases, + error: errorDatabases, + isFetching: isLoadingDatabases, + } = useListDatabasesQuery({ saved: false }); + + const { + data: schemas, + error: errorSchemas, + isFetching: isLoadingSchemas, + } = useListDatabaseSchemasQuery(isNotNull(dbId) ? { id: dbId } : skipToken); + + const { + data: tables, + error: errorTables, + isFetching: isLoadingTables, + } = useListDatabaseSchemaTablesQuery( + isNotNull(dbId) && isNotNull(schemaName) + ? { id: dbId, schema: schemaName } + : skipToken, + ); + + const selectedDbItem = useMemo( + () => getDbItem(databases?.data, dbId), + [databases, dbId], + ); + + const selectedSchemaItem = useMemo( + () => getSchemaItem(schemaName), + [schemaName], + ); + + const selectedTableItem = useMemo( + () => getTableItem(tables, tableId), + [tables, tableId], + ); + + const handleFolderSelect = useCallback( + ({ folder }: { folder: NotebookDataPickerFolderItem }) => { + if (folder.model === "database") { + if (dbId === folder.id) { + setSchemaName(schemas?.length === 1 ? schemas[0] : undefined); + } else { + setDbId(folder.id); + setSchemaName(undefined); + } + setTableId(undefined); + } + + if (folder.model === "schema") { + setSchemaName(folder.id); + setTableId(undefined); + } + }, + [dbId, schemas], + ); + + const handleItemSelect = useCallback( + (item: NotebookDataPickerValueItem) => { + setTableId(item.id); + onChange(item); + }, + [setTableId, onChange], + ); + + return ( + + + {!databaseId && ( + handleFolderSelect({ folder })} + /> + )} + + {isNotNull(dbId) && ( + handleFolderSelect({ folder })} + /> + )} + + {isNotNull(schemaName) && ( + + )} + + + ); +}; diff --git a/frontend/src/metabase/common/components/DataPicker/components/index.ts b/frontend/src/metabase/common/components/DataPicker/components/index.ts new file mode 100644 index 0000000000000..01bd5c5e86c90 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/components/index.ts @@ -0,0 +1 @@ +export * from "./DataPickerModal"; diff --git a/frontend/src/metabase/common/components/DataPicker/hooks/index.ts b/frontend/src/metabase/common/components/DataPicker/hooks/index.ts new file mode 100644 index 0000000000000..294e9c4f07214 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/hooks/index.ts @@ -0,0 +1,2 @@ +export * from "./useAutoSelectOnlyItem"; +export * from "./useAvailableData"; diff --git a/frontend/src/metabase/common/components/DataPicker/hooks/useAutoSelectOnlyItem.ts b/frontend/src/metabase/common/components/DataPicker/hooks/useAutoSelectOnlyItem.ts new file mode 100644 index 0000000000000..c8abb351d8f90 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/hooks/useAutoSelectOnlyItem.ts @@ -0,0 +1,37 @@ +import { useEffect } from "react"; +import { useLatest } from "react-use"; + +interface Props { + disabled: boolean; + items: Item[] | undefined; + onChange: (item: Item) => void; +} + +/** + * Automatically selects the only item on the list. + * Does nothing if there's 0 items or more than 1. + * + * @returns true when there's only 1 item. + */ +export const useAutoSelectOnlyItem = ({ + disabled, + items, + onChange, +}: Props): boolean => { + // use ref to avoid triggering the effect too often + const onChangeRef = useLatest(onChange); + const hasOnly1Item = items?.length === 1; + const onlyItem = hasOnly1Item ? items[0] : undefined; + + useEffect( + function autoSelectOnlyItem() { + if (!disabled && onlyItem) { + onChangeRef.current(onlyItem); + } + }, + [disabled, onlyItem, onChangeRef], + ); + + // let consumer component know when to not render itself + return hasOnly1Item; +}; diff --git a/frontend/src/metabase/common/components/DataPicker/hooks/useAvailableData.ts b/frontend/src/metabase/common/components/DataPicker/hooks/useAvailableData.ts new file mode 100644 index 0000000000000..cf0b227306dc9 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/hooks/useAvailableData.ts @@ -0,0 +1,22 @@ +import { useSearchQuery } from "metabase/api"; +import type { DatabaseId } from "metabase-types/api"; + +interface Props { + databaseId?: DatabaseId; +} + +export const useAvailableData = ({ databaseId }: Props = {}) => { + const { data } = useSearchQuery({ + limit: 0, + models: ["card"], + table_db_id: databaseId, + }); + const availableModels = data?.available_models ?? []; + const hasModels = availableModels.includes("dataset"); + const hasQuestions = availableModels.includes("card"); + + return { + hasModels, + hasQuestions, + }; +}; diff --git a/frontend/src/metabase/common/components/DataPicker/index.ts b/frontend/src/metabase/common/components/DataPicker/index.ts new file mode 100644 index 0000000000000..b793f517ee62c --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/index.ts @@ -0,0 +1,3 @@ +export * from "./components"; +export * from "./types"; +export * from "./utils"; diff --git a/frontend/src/metabase/common/components/DataPicker/types.ts b/frontend/src/metabase/common/components/DataPicker/types.ts new file mode 100644 index 0000000000000..83fcc90018d69 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/types.ts @@ -0,0 +1,66 @@ +import type { + CardId, + Collection, + DatabaseId, + SchemaName, + TableId, +} from "metabase-types/api"; + +import type { EntityPickerModalOptions } from "../EntityPicker"; +import type { QuestionPickerOptions } from "../QuestionPicker"; + +export type CollectionItem = { + id: Collection["id"]; + name: Collection["name"]; + model: "collection"; +}; + +export type DatabaseItem = { + id: DatabaseId; + name: string; + model: "database"; +}; + +export type SchemaItem = { + id: SchemaName; + name: string; + model: "schema"; +}; + +export type TableItem = { + id: TableId; + name: string; + model: "table"; +}; + +export type QuestionItem = { + id: CardId; + name: string; + model: "card"; +}; + +export type ModelItem = { + id: CardId; + name: string; + model: "dataset"; +}; + +export type TablePickerValue = { + id: TableId; + name: string; + model: "table"; + db_id: DatabaseId; + schema: SchemaName; +}; + +export type DataPickerValue = TablePickerValue | QuestionItem | ModelItem; + +export type NotebookDataPickerFolderItem = + | CollectionItem + | DatabaseItem + | SchemaItem; + +export type NotebookDataPickerValueItem = TableItem | QuestionItem | ModelItem; + +export type DataPickerModalOptions = EntityPickerModalOptions & + QuestionPickerOptions; diff --git a/frontend/src/metabase/common/components/DataPicker/utils.ts b/frontend/src/metabase/common/components/DataPicker/utils.ts new file mode 100644 index 0000000000000..d837fa31764a3 --- /dev/null +++ b/frontend/src/metabase/common/components/DataPicker/utils.ts @@ -0,0 +1,209 @@ +import { humanize, titleize } from "metabase/lib/formatting"; +import { isNullOrUndefined } from "metabase/lib/types"; +import * as Lib from "metabase-lib"; +import TableEntity from "metabase-lib/v1/metadata/Table"; +import { getSchemaName } from "metabase-lib/v1/metadata/utils/schema"; +import type { + Card, + CollectionItem, + Database, + DatabaseId, + SchemaName, + SearchModel, + Table, + TableId, +} from "metabase-types/api"; + +import type { QuestionPickerItem } from "../QuestionPicker"; + +import type { + DataPickerValue, + ModelItem, + NotebookDataPickerFolderItem, + NotebookDataPickerValueItem, + QuestionItem, + TablePickerValue, +} from "./types"; + +export const generateKey = ( + dbItem: NotebookDataPickerFolderItem | null, + schemaItem: NotebookDataPickerFolderItem | null, + tableItem: NotebookDataPickerValueItem | null, +) => { + return [dbItem?.id, schemaItem?.id, tableItem?.id].join("-"); +}; + +export const dataPickerValueFromCard = (card: Card): DataPickerValue => { + return { + id: card.id, + name: card.name, + model: card.type === "model" ? "dataset" : "card", + }; +}; + +export const dataPickerValueFromTable = ( + table: Table | TableEntity | null, +): TablePickerValue | undefined => { + if (table === null) { + return undefined; + } + + // Temporary, for backward compatibility in DataStep, until entity framework is no more + if (table instanceof TableEntity) { + return tablePickerValueFromTableEntity(table); + } + + return { + db_id: table.db_id, + id: table.id, + model: "table", + name: table.display_name, + schema: table.schema, + }; +}; + +export const dataPickerValueFromJoinable = ( + query: Lib.Query, + stageIndex: number, + joinable: Lib.Joinable, +): DataPickerValue | undefined => { + const pickerInfo = Lib.pickerInfo(query, joinable); + const displayInfo = Lib.displayInfo(query, stageIndex, joinable); + + if (!pickerInfo) { + return undefined; + } + + if (typeof pickerInfo.cardId === "number") { + return { + id: pickerInfo.cardId, + name: displayInfo.displayName, + model: displayInfo.isModel ? "dataset" : "card", + }; + } + + return { + id: pickerInfo.tableId, + name: displayInfo.displayName, + model: "table", + db_id: pickerInfo.databaseId, + schema: getSchemaName(displayInfo.schema), + }; +}; + +const tablePickerValueFromTableEntity = ( + table: TableEntity, +): TablePickerValue => { + // In DBs without schemas, API will use an empty string to indicate the default, virtual schema + const NO_SCHEMA_FALLBACK = ""; + + return { + db_id: table.db_id, + id: table.id, + model: "table", + name: table.display_name, + schema: table.schema_name ?? table.schema?.name ?? NO_SCHEMA_FALLBACK, + }; +}; + +export const getDbItem = ( + databases: Database[] | undefined, + dbId: DatabaseId | undefined, +): NotebookDataPickerFolderItem | null => { + if (typeof dbId === "undefined") { + return null; + } + + const database = databases?.find(db => db.id === dbId); + const name = database?.name ?? ""; + + return { model: "database", id: dbId, name }; +}; + +export const getSchemaItem = ( + schemaName: SchemaName | undefined, +): NotebookDataPickerFolderItem | null => { + if (typeof schemaName === "undefined") { + return null; + } + + const name = getSchemaDisplayName(schemaName); + + return { model: "schema", id: schemaName, name }; +}; + +export const getTableItem = ( + tables: Table[] | undefined, + tableId: TableId | undefined, +): NotebookDataPickerValueItem | null => { + if (typeof tableId === "undefined") { + return null; + } + + const table = tables?.find(db => db.id === tableId); + const name = table?.name ?? ""; + + return { model: "table", id: tableId, name }; +}; + +export const getSchemaDisplayName = (schemaName: SchemaName | undefined) => { + if (typeof schemaName === "undefined") { + return ""; + } + + return titleize(humanize(schemaName)); +}; + +export const isModelItem = ( + value: DataPickerValue | undefined, +): value is ModelItem => { + return value?.model === "dataset"; +}; + +export const isQuestionItem = ( + value: DataPickerValue | undefined, +): value is QuestionItem => { + return value?.model === "card"; +}; + +export const isTableItem = ( + value: DataPickerValue | undefined, +): value is TablePickerValue => { + return value?.model === "table"; +}; + +export const isValidValueItem = (model: SearchModel): boolean => { + return ["dataset", "card", "table"].includes(model); +}; + +export const createShouldShowItem = (databaseId?: DatabaseId) => { + return (item: QuestionPickerItem) => { + if (item.model === "collection") { + const below = item.below ?? []; + const here = item.here ?? []; + const contents = [...below, ...here]; + const hasQuestionsOrModels = + contents.includes("card") || contents.includes("dataset"); + + if (item.id !== "root" && !item.is_personal && !hasQuestionsOrModels) { + return false; + } + } + + if ( + isNullOrUndefined(databaseId) || + !hasDatabaseId(item) || + isNullOrUndefined(item.database_id) + ) { + return true; + } + + return item.database_id === databaseId; + }; +}; + +const hasDatabaseId = ( + value: unknown, +): value is Pick => { + return typeof value === "object" && value != null && "database_id" in value; +}; diff --git a/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.tsx b/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.tsx index e407e05e9f952..224c5ecbde358 100644 --- a/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.tsx +++ b/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.tsx @@ -8,11 +8,11 @@ import { BULK_ACTIONS_Z_INDEX } from "metabase/components/BulkActionBar"; import { useModalOpen } from "metabase/hooks/use-modal-open"; import { Modal } from "metabase/ui"; import type { + RecentItem, SearchModel, - SearchResultId, SearchRequest, SearchResult, - RecentItem, + SearchResultId, } from "metabase-types/api"; import type { @@ -35,7 +35,6 @@ import { TabsView } from "./TabsView"; export type EntityPickerModalOptions = { showSearch?: boolean; hasConfirmButtons?: boolean; - allowCreateNew?: boolean; confirmButtonText?: string; cancelButtonText?: string; hasRecents?: boolean; @@ -44,7 +43,6 @@ export type EntityPickerModalOptions = { export const defaultOptions: EntityPickerModalOptions = { showSearch: true, hasConfirmButtons: true, - allowCreateNew: true, hasRecents: true, }; @@ -55,7 +53,7 @@ export interface EntityPickerModalProps { title?: string; selectedItem: Item | null; initialValue?: Partial; - onConfirm: () => void; + onConfirm?: () => void; onItemSelect: (item: Item) => void; canSelectItem: boolean; onClose: () => void; @@ -101,13 +99,12 @@ export function EntityPickerModal< ); const hydratedOptions = useMemo( - () => ({ - ...defaultOptions, - ...options, - }), + () => ({ ...defaultOptions, ...options }), [options], ); + assertValidProps(hydratedOptions, onConfirm); + const { open } = useModalOpen(); const tabModels = useMemo( @@ -213,7 +210,7 @@ export function EntityPickerModal< ) : ( {tabs[0].element} )} - {!!hydratedOptions.hasConfirmButtons && ( + {!!hydratedOptions.hasConfirmButtons && onConfirm && ( ); } + +const assertValidProps = ( + options: EntityPickerModalOptions, + onConfirm: (() => void) | undefined, +) => { + if (options.hasConfirmButtons && !onConfirm) { + throw new Error( + "onConfirm prop is required when hasConfirmButtons is true", + ); + } +}; diff --git a/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.unit.spec.tsx b/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.unit.spec.tsx index cf02ced48b2ce..9ed94c7fd00c8 100644 --- a/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.unit.spec.tsx +++ b/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerModal/EntityPickerModal.unit.spec.tsx @@ -30,7 +30,7 @@ interface SetupOpts { onItemSelect?: () => void; onClose?: () => void; onConfirm?: () => void; - tabs?: [EntityTab, ...EntityTab[]]; + tabs?: EntityTab[]; options?: EntityPickerModalOptions; selectedItem?: null | TypeWithModel; actionButtons?: JSX.Element[]; @@ -62,7 +62,7 @@ const setup = ({ title = "Pick a thing", onItemSelect = jest.fn(), onClose = jest.fn(), - onConfirm = jest.fn(), + onConfirm, tabs = [TEST_CARD_TAB], selectedItem = null, recentItems = [], @@ -79,9 +79,9 @@ const setup = ({ onItemSelect={onItemSelect} canSelectItem={true} onClose={onClose} + onConfirm={onConfirm} tabs={tabs} selectedItem={selectedItem} - onConfirm={onConfirm} recentFilter={recentFilter} {...rest} />, @@ -92,13 +92,31 @@ describe("EntityPickerModal", () => { afterAll(() => { jest.restoreAllMocks(); }); + + it("should throw when options.hasConfirmButtons is true but onConfirm prop is missing", async () => { + expect(() => { + setup({ + options: { + hasConfirmButtons: true, + }, + onConfirm: undefined, + }); + }).toThrow("onConfirm prop is required when hasConfirmButtons is true"); + }); + it("should render a picker", async () => { - setup({}); + setup({ + onConfirm: jest.fn(), + }); + expect(await screen.findByText("Test picker foo")).toBeInTheDocument(); }); it("should render a search bar by default and show confirmation button", async () => { - setup(); + setup({ + onConfirm: jest.fn(), + }); + expect(await screen.findByPlaceholderText("Search…")).toBeInTheDocument(); expect( await screen.findByRole("button", { name: "Select" }), @@ -110,13 +128,15 @@ describe("EntityPickerModal", () => { options: { showSearch: false, }, + onConfirm: jest.fn(), }); + expect(screen.queryByPlaceholderText("Search…")).not.toBeInTheDocument(); }); it("should show a tab list when more than 1 tab is supplied", async () => { - const tabs: [EntityTab, ...EntityTab[]] = - [ + setup({ + tabs: [ TEST_CARD_TAB, { icon: "folder", @@ -124,9 +144,8 @@ describe("EntityPickerModal", () => { model: "table", element: , }, - ]; - setup({ - tabs, + ], + onConfirm: jest.fn(), }); const tabList = await screen.findByRole("tablist"); @@ -171,10 +190,10 @@ describe("EntityPickerModal", () => { fetchMock.get("path:/api/user/recipients", { data: [] }); const onItemSelect = jest.fn(); - const onConfirm = jest.fn(); + setup({ onItemSelect, - onConfirm, + onConfirm: jest.fn(), }); await userEvent.type(await screen.findByPlaceholderText("Search…"), "My ", { @@ -202,7 +221,10 @@ describe("EntityPickerModal", () => { , ]; - setup({ actionButtons }); + setup({ + actionButtons, + onConfirm: jest.fn(), + }); expect( await screen.findByRole("button", { name: "Click Me" }), @@ -248,7 +270,7 @@ describe("EntityPickerModal", () => { ]; it("should not show a recents tab when there are no recent items", async () => { - setup({}); + setup({ onConfirm: jest.fn() }); await screen.findByText("Test picker foo"); @@ -256,7 +278,10 @@ describe("EntityPickerModal", () => { }); it("should show a recents tab when there are recent items", async () => { - setup({ recentItems }); + setup({ + recentItems, + onConfirm: jest.fn(), + }); expect( await screen.findByRole("tab", { name: /Recents/ }), @@ -269,6 +294,7 @@ describe("EntityPickerModal", () => { recentItems, defaultToRecentTab: false, initialValue: { model: "card" }, + onConfirm: jest.fn(), }); expect( @@ -278,13 +304,17 @@ describe("EntityPickerModal", () => { }); it("should group recents by time", async () => { - setup({ recentItems }); + setup({ + recentItems, + onConfirm: jest.fn(), + }); expect(await screen.findByText("Earlier")).toBeInTheDocument(); }); it("should filter out irrelevant models", async () => { setup({ + onConfirm: jest.fn(), recentItems, tabs: [TEST_CARD_TAB, TEST_TABLE_TAB], }); @@ -296,6 +326,7 @@ describe("EntityPickerModal", () => { it("should accept an arbitrary filter", async () => { setup({ + onConfirm: jest.fn(), recentItems, recentFilter: items => items.filter(item => !item.description?.includes("invisible")), diff --git a/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerSearch/EntityPickerSearch.tsx b/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerSearch/EntityPickerSearch.tsx index 5ed4ad40757af..99b61598fc449 100644 --- a/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerSearch/EntityPickerSearch.tsx +++ b/frontend/src/metabase/common/components/EntityPicker/components/EntityPickerSearch/EntityPickerSearch.tsx @@ -16,7 +16,7 @@ import type { import type { TypeWithModel } from "../../types"; import { DelayedLoadingSpinner } from "../LoadingSpinner"; -import { ResultItem, ChunkyList } from "../ResultItem"; +import { ChunkyList, ResultItem } from "../ResultItem"; import { getSearchTabText } from "./utils"; diff --git a/frontend/src/metabase/common/components/EntityPicker/components/ItemList/ItemList.tsx b/frontend/src/metabase/common/components/EntityPicker/components/ItemList/ItemList.tsx index 171d2067c6ae9..5f8673c7b028a 100644 --- a/frontend/src/metabase/common/components/EntityPicker/components/ItemList/ItemList.tsx +++ b/frontend/src/metabase/common/components/EntityPicker/components/ItemList/ItemList.tsx @@ -20,11 +20,12 @@ interface ItemListProps< items?: Item[] | null; isLoading?: boolean; error?: unknown; - onClick: (val: Item) => void; + onClick: (item: Item) => void; selectedItem: Item | null; isFolder: (item: Item) => boolean; isCurrentLevel: boolean; shouldDisableItem?: (item: Item) => boolean; + shouldShowItem?: (item: Item) => boolean; } export const ItemList = < @@ -40,20 +41,23 @@ export const ItemList = < isFolder, isCurrentLevel, shouldDisableItem, + shouldShowItem, }: ItemListProps) => { + const filteredItems = + items && shouldShowItem ? items.filter(shouldShowItem) : items; const activeItemIndex = useMemo(() => { - if (!items) { + if (!filteredItems) { return -1; } - return items.findIndex(item => isSelectedItem(item, selectedItem)); - }, [items, selectedItem]); + return filteredItems.findIndex(item => isSelectedItem(item, selectedItem)); + }, [filteredItems, selectedItem]); if (error) { return ; } - if (isLoading && !items) { + if (isLoading && !filteredItems) { return (
@@ -63,13 +67,13 @@ export const ItemList = < ); } - if (!items || !items.length) { + if (!filteredItems || !filteredItems.length) { return null; } return ( - {items.map((item: Item) => ( + {filteredItems.map((item: Item) => (
; listResolver: ComponentType>; shouldDisableItem?: (item: Item) => boolean; + shouldShowItem?: (item: Item) => boolean; } export function NestedItemPicker< @@ -46,6 +47,7 @@ export function NestedItemPicker< isFolder, listResolver: ListResolver, shouldDisableItem, + shouldShowItem, }: NestedItemPickerProps) { const handleClick = (item: Item) => { if (isFolder(item)) { @@ -84,6 +86,7 @@ export function NestedItemPicker< onClick={(item: Item) => handleClick(item)} isCurrentLevel={isCurrentLevel} shouldDisableItem={shouldDisableItem} + shouldShowItem={shouldShowItem} isFolder={isFolder} /> diff --git a/frontend/src/metabase/common/components/EntityPicker/components/NestedItemPicker/index.ts b/frontend/src/metabase/common/components/EntityPicker/components/NestedItemPicker/index.ts index dd2ef6be63444..616899bdc89cb 100644 --- a/frontend/src/metabase/common/components/EntityPicker/components/NestedItemPicker/index.ts +++ b/frontend/src/metabase/common/components/EntityPicker/components/NestedItemPicker/index.ts @@ -1 +1,2 @@ export * from "./NestedItemPicker"; +export * from "./NestedItemPicker.styled"; diff --git a/frontend/src/metabase/common/components/EntityPicker/components/ResultItem/ResultItem.tsx b/frontend/src/metabase/common/components/EntityPicker/components/ResultItem/ResultItem.tsx index a00d87aecf554..b6f54bd697448 100644 --- a/frontend/src/metabase/common/components/EntityPicker/components/ResultItem/ResultItem.tsx +++ b/frontend/src/metabase/common/components/EntityPicker/components/ResultItem/ResultItem.tsx @@ -38,9 +38,11 @@ export const ResultItem = ({ return ( diff --git a/frontend/src/metabase/common/components/EntityPicker/types.ts b/frontend/src/metabase/common/components/EntityPicker/types.ts index c330cefae33ec..77b04b5a5eca3 100644 --- a/frontend/src/metabase/common/components/EntityPicker/types.ts +++ b/frontend/src/metabase/common/components/EntityPicker/types.ts @@ -45,6 +45,7 @@ export type ListProps< isCurrentLevel: boolean; options: Options; shouldDisableItem?: (item: Item) => boolean; + shouldShowItem?: (item: Item) => boolean; }; export type FilterItemsInPersonalCollection = "only" | "exclude"; diff --git a/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPicker.tsx b/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPicker.tsx index 0760014a2137b..de2e801e8bb63 100644 --- a/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPicker.tsx +++ b/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPicker.tsx @@ -11,8 +11,8 @@ import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; import { useSelector } from "metabase/lib/redux"; import { getUserPersonalCollectionId } from "metabase/selectors/user"; import type { - ListCollectionItemsRequest, CollectionItemModel, + ListCollectionItemsRequest, } from "metabase-types/api"; import { CollectionItemPickerResolver } from "../../CollectionPicker/components/CollectionItemPickerResolver"; @@ -22,20 +22,21 @@ import { NestedItemPicker, type PickerState, } from "../../EntityPicker"; -import type { QuestionPickerOptions, QuestionPickerItem } from "../types"; +import type { QuestionPickerItem, QuestionPickerOptions } from "../types"; import { getCollectionIdPath, getStateFromIdPath, isFolder } from "../utils"; export const defaultOptions: QuestionPickerOptions = { showPersonalCollections: true, showRootCollection: true, - allowCreateNew: false, hasConfirmButtons: false, }; + interface QuestionPickerProps { onItemSelect: (item: QuestionPickerItem) => void; initialValue?: Pick; options: QuestionPickerOptions; models?: CollectionItemModel[]; + shouldShowItem?: (item: QuestionPickerItem) => boolean; } const useGetInitialCollection = ( @@ -75,6 +76,7 @@ export const QuestionPicker = ({ initialValue, options, models = ["dataset", "card"], + shouldShowItem, }: QuestionPickerProps) => { const [path, setPath] = useState< PickerState @@ -169,6 +171,7 @@ export const QuestionPicker = ({ onItemSelect={handleItemSelect} path={path} listResolver={CollectionItemPickerResolver} + shouldShowItem={shouldShowItem} /> ); }; diff --git a/frontend/src/metabase/common/components/QuestionPicker/utils.ts b/frontend/src/metabase/common/components/QuestionPicker/utils.ts index fd2abf8c87d9e..dca7854974a28 100644 --- a/frontend/src/metabase/common/components/QuestionPicker/utils.ts +++ b/frontend/src/metabase/common/components/QuestionPicker/utils.ts @@ -3,8 +3,8 @@ import _ from "underscore"; import { PERSONAL_COLLECTIONS } from "metabase/entities/collections"; import type { CollectionId, - ListCollectionItemsRequest, CollectionItemModel, + ListCollectionItemsRequest, } from "metabase-types/api"; import type { PickerState } from "../EntityPicker"; diff --git a/frontend/src/metabase/containers/DataPicker/types.ts b/frontend/src/metabase/containers/DataPicker/types.ts index a7fac24ab179c..0974bf3d38cac 100644 --- a/frontend/src/metabase/containers/DataPicker/types.ts +++ b/frontend/src/metabase/containers/DataPicker/types.ts @@ -5,7 +5,7 @@ import type Table from "metabase-lib/v1/metadata/Table"; import type { CollectionId, DatabaseId, - SchemaId, + SchemaName, TableId, } from "metabase-types/api"; @@ -14,7 +14,7 @@ export type DataPickerDataType = "models" | "raw-data" | "questions"; export type DataPickerValue = { type?: DataPickerDataType; databaseId?: DatabaseId; - schemaId?: SchemaId; + schemaId?: SchemaName; collectionId?: CollectionId; tableIds: TableId[]; }; diff --git a/frontend/src/metabase/entities/schemas.js b/frontend/src/metabase/entities/schemas.js index 1faec8cd591f0..9e8860a66d6bd 100644 --- a/frontend/src/metabase/entities/schemas.js +++ b/frontend/src/metabase/entities/schemas.js @@ -81,6 +81,10 @@ export default createEntity({ getObject: (state, { entityId }) => getMetadata(state).schema(entityId), }, + objectSelectors: { + getIcon: () => ({ name: "folder" }), + }, + reducer: (state = {}, { type, payload, error }) => { if (type === Questions.actionTypes.CREATE && !error) { const { question, status, data } = payload; diff --git a/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.styled.tsx b/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.styled.tsx index 9c3bcdf4788b6..302840560e332 100644 --- a/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.styled.tsx +++ b/frontend/src/metabase/query_builder/components/DataSelector/DataSelector.styled.tsx @@ -55,39 +55,6 @@ export const DataBucketList = styled(SelectList)` padding: ${space(0)} ${space(1)} 12px ${space(1)}; `; -export const CollectionDatasetSelectList = styled(SelectList)` - width: 300px; - max-width: 300px; - padding: 0.5rem; -`; - -Object.assign(CollectionDatasetSelectList, { Item: SelectList.Item }); - -export const CollectionDatasetAllDataLink = styled(SelectList.BaseItem)` - padding: 0.5rem; - - color: ${color("text-light")}; - font-weight: bold; - cursor: pointer; - - :hover { - color: ${color("brand")}; - } -`; - -const CollectionDatasetAllDataContent = styled.span` - display: flex; - align-items: center; - - .Icon { - margin-left: ${space(0)}; - } -`; - -Object.assign(CollectionDatasetAllDataLink, { - Content: CollectionDatasetAllDataContent, -}); - export const EmptyStateContainer = styled.div` width: 300px; padding: 80px 60px; diff --git a/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx b/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx index 72a6a93d9e7e5..9f4df1b03987e 100644 --- a/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/Notebook.tsx @@ -15,7 +15,7 @@ import type { State } from "metabase-types/store"; import { NotebookSteps } from "./NotebookSteps"; -interface NotebookOwnProps { +interface NotebookProps { className?: string; question: Question; isDirty: boolean; @@ -29,12 +29,6 @@ interface NotebookOwnProps { readOnly?: boolean; } -interface EntityLoaderProps { - sourceQuestion?: Question; -} - -type NotebookProps = NotebookOwnProps & EntityLoaderProps; - const Notebook = ({ className, updateQuestion, ...props }: NotebookProps) => { const { question, @@ -110,7 +104,7 @@ function getSourceQuestionId(question: Question) { // eslint-disable-next-line import/no-default-export -- deprecated usage export default _.compose( Questions.load({ - id: (state: State, { question }: NotebookOwnProps) => + id: (state: State, { question }: NotebookProps) => getSourceQuestionId(question), entityAlias: "sourceQuestion", loadingAndErrorWrapper: false, 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 365a4c2a4e015..aede442e5f891 100644 --- a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.tsx @@ -10,7 +10,6 @@ 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"; import type { @@ -20,13 +19,13 @@ import type { import ActionButton from "./ActionButton"; import { + PreviewButton, StepActionsContainer, StepBody, + StepButtonContainer, StepContent, StepHeader, - StepButtonContainer, StepRoot, - PreviewButton, } from "./NotebookStep.styled"; import { STEP_UI } from "./steps"; @@ -36,7 +35,6 @@ function hasLargeButton(action: NotebookStepAction) { interface NotebookStepProps { step: INotebookStep; - sourceQuestion?: Question; isLastStep: boolean; isLastOpened: boolean; reportTimezone: string; @@ -47,7 +45,6 @@ interface NotebookStepProps { function NotebookStep({ step, - sourceQuestion, isLastStep, isLastOpened, reportTimezone, @@ -149,7 +146,6 @@ function NotebookStep({ step={step} query={step.query} stageIndex={step.stageIndex} - sourceQuestion={sourceQuestion} updateQuery={updateQuery} isLastOpened={isLastOpened} reportTimezone={reportTimezone} diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.unit.spec.tsx b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.unit.spec.tsx index c46277abe7de2..9397e089ecd3f 100644 --- a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/NotebookStep.unit.spec.tsx @@ -2,13 +2,14 @@ import userEvent from "@testing-library/user-event"; import { setupDatabasesEndpoints, + setupRecentViewsEndpoints, setupSearchEndpoints, } from "__support__/server-mocks"; import { renderWithProviders, screen } from "__support__/ui"; import type Question from "metabase-lib/v1/Question"; import { createSampleDatabase } from "metabase-types/api/mocks/presets"; -import { createMockNotebookStep, DEFAULT_QUESTION } from "../test-utils"; +import { createMockNotebookStep } from "../test-utils"; import type { NotebookStep as INotebookStep, NotebookStepType } from "../types"; import NotebookStep from "./NotebookStep"; @@ -18,20 +19,17 @@ type SetupOpts = { question?: Question; }; -function setup({ - step = createMockNotebookStep(), - question = DEFAULT_QUESTION, -}: SetupOpts = {}) { +function setup({ step = createMockNotebookStep() }: SetupOpts = {}) { const openStep = jest.fn(); const updateQuery = jest.fn(); setupDatabasesEndpoints([createSampleDatabase()]); setupSearchEndpoints([]); + setupRecentViewsEndpoints([]); renderWithProviders( { + const dispatch = useDispatch(); const { stageIndex } = step; - const question = step.question; - const collectionId = question.collectionId(); - const databaseId = Lib.databaseID(query); + const questionRef = useLatest(question); + const metadata = question.metadata(); + const tableId = Lib.sourceTableOrCardId(query); - const table = tableId ? Lib.tableOrCardMetadata(query, tableId) : null; + const table = metadata.table(tableId); + const tableMetadata = tableId + ? Lib.tableOrCardMetadata(query, tableId) + : null; + + const sourceCardId = getQuestionIdFromVirtualTableId(tableId); + const { data: sourceCard } = useGetCardQuery( + sourceCardId ? { id: sourceCardId } : skipToken, + ); - const pickerLabel = table - ? Lib.displayInfo(query, stageIndex, table).displayName + const [isDataPickerOpen, setIsDataPickerOpen] = useState(!tableMetadata); + + const pickerLabel = tableMetadata + ? Lib.displayInfo(query, stageIndex, tableMetadata).displayName : t`Pick your starting data`; const isRaw = useMemo(() => { @@ -38,20 +60,41 @@ export const DataStep = ({ ); }, [query, stageIndex]); - const canSelectTableColumns = table && isRaw && !readOnly; + const canSelectTableColumns = tableMetadata && isRaw && !readOnly; + + const handleTableChange = async (tableId: TableId) => { + // we need to populate question metadata with selected table + await dispatch(Tables.actions.fetchMetadata({ id: tableId })); - const handleTableSelect = (tableId: TableId, databaseId: DatabaseId) => { - const metadata = question.metadata(); - const metadataProvider = Lib.metadataProvider(databaseId, metadata); + if (typeof tableId === "string") { + await dispatch( + Questions.actions.fetch({ + id: getQuestionIdFromVirtualTableId(tableId), + }), + ); + } + + // using questionRef because question is most likely stale by now + const metadata = questionRef.current.metadata(); + const table = checkNotNull(metadata.table(tableId)); + const metadataProvider = Lib.metadataProvider(table.db_id, metadata); const nextTable = Lib.tableOrCardMetadata(metadataProvider, tableId); updateQuery(Lib.queryFromTableOrCardMetadata(metadataProvider, nextTable)); }; + const value = useMemo(() => { + if (sourceCardId && sourceCard) { + return dataPickerValueFromCard(sourceCard); + } + + return dataPickerValueFromTable(table); + }, [sourceCard, sourceCardId, table]); + return ( - {pickerLabel}} - /> + setIsDataPickerOpen(true)}> + {pickerLabel} + + + {isDataPickerOpen && ( + setIsDataPickerOpen(false)} + /> + )} ); diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/DataStep/DataStep.unit.spec.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/DataStep/DataStep.unit.spec.tsx index 6fff63d9f6de1..e0788bfeaaa3e 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/DataStep/DataStep.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/DataStep/DataStep.unit.spec.tsx @@ -2,9 +2,10 @@ import userEvent from "@testing-library/user-event"; import { setupDatabasesEndpoints, + setupRecentViewsEndpoints, setupSearchEndpoints, } from "__support__/server-mocks"; -import { renderWithProviders, screen } from "__support__/ui"; +import { renderWithProviders, screen, within } from "__support__/ui"; import * as Lib from "metabase-lib"; import { columnFinder, @@ -50,6 +51,7 @@ const setup = async ( const updateQuery = jest.fn(); setupDatabasesEndpoints([createSampleDatabase()]); setupSearchEndpoints([]); + setupRecentViewsEndpoints([]); renderWithProviders( { }; describe("DataStep", () => { + const scrollBy = HTMLElement.prototype.scrollBy; + const getBoundingClientRect = HTMLElement.prototype.getBoundingClientRect; + + beforeAll(() => { + HTMLElement.prototype.scrollBy = jest.fn(); + // needed for @tanstack/react-virtual, see https://github.com/TanStack/virtual/issues/29#issuecomment-657519522 + HTMLElement.prototype.getBoundingClientRect = jest + .fn() + .mockReturnValue({ height: 1, width: 1 }); + }); + + afterAll(() => { + HTMLElement.prototype.scrollBy = scrollBy; + HTMLElement.prototype.getBoundingClientRect = getBoundingClientRect; + + jest.resetAllMocks(); + }); + it("should render without a table selected", async () => { await setupEmptyQuery(); - expect(screen.getByText("Pick your starting data")).toBeInTheDocument(); + const modal = await screen.findByTestId("entity-picker-modal"); + expect( + within(modal).getByText("Pick your starting data"), + ).toBeInTheDocument(); - // Ensure the table picker is not open - expect(screen.getByText("Sample Database")).toBeInTheDocument(); - expect(screen.getByText("Orders")).toBeInTheDocument(); - expect(screen.getByText("Products")).toBeInTheDocument(); - expect(screen.getByText("People")).toBeInTheDocument(); + // Ensure the table picker not open + expect(within(modal).getByText("Orders")).toBeInTheDocument(); + expect(within(modal).getByText("Products")).toBeInTheDocument(); + expect(within(modal).getByText("People")).toBeInTheDocument(); }); it("should render with a selected table", async () => { diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/Join/Join.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/Join/Join.tsx index b77798b11b83d..5396c955bf3c6 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/Join/Join.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/Join/Join.tsx @@ -12,7 +12,6 @@ interface JoinProps { joinPosition: number; color: string; isReadOnly: boolean; - isModelDataSource: boolean; onJoinChange: (newJoin: Lib.Join) => void; onQueryChange: (newQuery: Lib.Query) => void; } @@ -24,7 +23,6 @@ export function Join({ joinPosition, color, isReadOnly, - isModelDataSource, onJoinChange, onQueryChange, }: JoinProps) { @@ -45,7 +43,6 @@ export function Join({ initialStrategy={draftStrategy} initialRhsTable={draftRhsTable} isReadOnly={isReadOnly} - isModelDataSource={isModelDataSource} onJoinChange={handleJoinChange} /> ); @@ -59,7 +56,6 @@ export function Join({ joinPosition={joinPosition} color={color} isReadOnly={isReadOnly} - isModelDataSource={isModelDataSource} onJoinChange={handleJoinChange} onQueryChange={onQueryChange} onDraftRhsTableChange={setDraftRhsTable} diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinComplete/JoinComplete.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinComplete/JoinComplete.tsx index 05870f7516061..f4a83e546568d 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinComplete/JoinComplete.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinComplete/JoinComplete.tsx @@ -11,7 +11,7 @@ import { JoinStrategyPicker } from "../JoinStrategyPicker"; import { JoinTableColumnPicker } from "../JoinTableColumnPicker"; import { JoinTablePicker } from "../JoinTablePicker"; -import { JoinConditionCell, JoinCell } from "./JoinComplete.styled"; +import { JoinCell, JoinConditionCell } from "./JoinComplete.styled"; interface JoinCompleteProps { query: Lib.Query; @@ -20,7 +20,6 @@ interface JoinCompleteProps { joinPosition: number; color: string; isReadOnly: boolean; - isModelDataSource: boolean; onJoinChange: (newJoin: Lib.Join) => void; onQueryChange: (newQuery: Lib.Query) => void; onDraftRhsTableChange: (newTable: Lib.Joinable) => void; @@ -33,7 +32,6 @@ export function JoinComplete({ joinPosition, color, isReadOnly, - isModelDataSource, onJoinChange, onQueryChange, onDraftRhsTableChange, @@ -113,11 +111,11 @@ export function JoinComplete({ /> void; } @@ -31,7 +30,6 @@ export function JoinDraft({ initialStrategy, initialRhsTable, isReadOnly, - isModelDataSource, onJoinChange, }: JoinDraftProps) { const [strategy, setStrategy] = useState( @@ -103,11 +101,11 @@ export function JoinDraft({ /> { const newQuery = Lib.join(query, stageIndex, newJoin); @@ -44,7 +42,6 @@ export function JoinStep({ joinPosition={itemIndex} color={color} isReadOnly={isReadOnly} - isModelDataSource={isModelDataSource} onJoinChange={handleUpdateJoin} onQueryChange={updateQuery} /> @@ -54,7 +51,6 @@ export function JoinStep({ stageIndex={stageIndex} color={color} isReadOnly={isReadOnly} - isModelDataSource={isModelDataSource} onJoinChange={handleAddJoin} /> ); diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinStep.unit.spec.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinStep.unit.spec.tsx index ae1b30893209a..081ce79d82903 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinStep.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinStep.unit.spec.tsx @@ -4,6 +4,7 @@ import { useState } from "react"; import { createMockMetadata } from "__support__/metadata"; import { setupDatabasesEndpoints, + setupRecentViewsEndpoints, setupSearchEndpoints, } from "__support__/server-mocks"; import { createMockEntitiesState } from "__support__/store"; @@ -122,6 +123,7 @@ function setup(step = createMockNotebookStep(), { readOnly = false } = {}) { setupDatabasesEndpoints(DATABASES); setupSearchEndpoints([createMockCollectionItem(MODEL)]); + setupRecentViewsEndpoints([]); function Wrapper() { const [query, setQuery] = useState(step.query); @@ -187,6 +189,24 @@ function setup(step = createMockNotebookStep(), { readOnly = false } = {}) { } describe("Notebook Editor > Join Step", () => { + const scrollBy = HTMLElement.prototype.scrollBy; + const getBoundingClientRect = HTMLElement.prototype.getBoundingClientRect; + + beforeAll(() => { + HTMLElement.prototype.scrollBy = jest.fn(); + // needed for @tanstack/react-virtual, see https://github.com/TanStack/virtual/issues/29#issuecomment-657519522 + HTMLElement.prototype.getBoundingClientRect = jest + .fn() + .mockReturnValue({ height: 1, width: 1 }); + }); + + afterAll(() => { + HTMLElement.prototype.scrollBy = scrollBy; + HTMLElement.prototype.getBoundingClientRect = getBoundingClientRect; + + jest.resetAllMocks(); + }); + it("should display a join correctly", () => { setup(createMockNotebookStep({ query: getJoinedQuery() })); @@ -205,14 +225,11 @@ describe("Notebook Editor > Join Step", () => { await userEvent.click( within(screen.getByLabelText("Right table")).getByRole("button"), ); - const popover = await screen.findByTestId("popover"); + const modal = await screen.findByTestId("entity-picker-modal"); - await waitFor(() => { - expect(within(popover).getByText("Sample Database")).toBeInTheDocument(); - }); - expect(within(popover).getByText("Products")).toBeInTheDocument(); - expect(within(popover).getByText("People")).toBeInTheDocument(); - expect(within(popover).getByText("Reviews")).toBeInTheDocument(); + expect(within(modal).getByText("Products")).toBeInTheDocument(); + expect(within(modal).getByText("People")).toBeInTheDocument(); + expect(within(modal).getByText("Reviews")).toBeInTheDocument(); }); it("should not allow picking a right table from another database", async () => { @@ -221,14 +238,10 @@ describe("Notebook Editor > Join Step", () => { await userEvent.click( within(screen.getByLabelText("Right table")).getByRole("button"), ); - const popover = await screen.findByTestId("popover"); - - // Go back to the database list - await userEvent.click(within(popover).getByText("Sample Database")); + const modal = await screen.findByTestId("entity-picker-modal"); - expect(within(popover).getByText("Sample Database")).toBeInTheDocument(); expect( - within(popover).queryByText(ANOTHER_DATABASE.name), + within(modal).queryByText(ANOTHER_DATABASE.name), ).not.toBeInTheDocument(); }); @@ -238,8 +251,8 @@ describe("Notebook Editor > Join Step", () => { await userEvent.click( within(screen.getByLabelText("Right table")).getByRole("button"), ); - const tablePicker = await screen.findByTestId("popover"); - await userEvent.click(await within(tablePicker).findByText("Reviews")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Reviews")); const lhsColumnPicker = await screen.findByTestId("lhs-column-picker"); @@ -266,8 +279,8 @@ describe("Notebook Editor > Join Step", () => { const pickerButton = within(rhsTablePicker).getByText("Products"); await userEvent.click(pickerButton); - const tablePicker = await screen.findByTestId("popover"); - await userEvent.click(await within(tablePicker).findByText("People")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("People")); const { conditions } = getRecentJoin(); const [condition] = conditions; @@ -286,8 +299,8 @@ describe("Notebook Editor > Join Step", () => { const pickerButton = within(rhsTablePicker).getByText("Products"); await userEvent.click(pickerButton); - const tablePicker = await screen.findByTestId("popover"); - await userEvent.click(await within(tablePicker).findByText("Reviews")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Reviews")); const lhsColumnPicker = await screen.findByTestId("lhs-column-picker"); await userEvent.click(within(lhsColumnPicker).getByText("Total")); @@ -338,11 +351,11 @@ describe("Notebook Editor > Join Step", () => { it("should automatically open RHS table picker", async () => { setup(); - const popover = await screen.findByTestId("popover"); + const modal = await screen.findByTestId("entity-picker-modal"); - expect(await within(popover).findByText("Products")).toBeInTheDocument(); - expect(within(popover).getByText("People")).toBeInTheDocument(); - expect(within(popover).getByText("Reviews")).toBeInTheDocument(); + expect(await within(modal).findByText("Products")).toBeInTheDocument(); + expect(within(modal).getByText("People")).toBeInTheDocument(); + expect(within(modal).getByText("Reviews")).toBeInTheDocument(); expect(screen.getByLabelText("Right table")).toHaveTextContent( "Pick data…", ); @@ -351,8 +364,8 @@ describe("Notebook Editor > Join Step", () => { it("should apply a suggested condition when table is selected", async () => { const { getRecentJoin } = setup(); - const popover = screen.getByTestId("popover"); - await userEvent.click(await within(popover).findByText("Products")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Products")); expect(await screen.findByLabelText("Left column")).toHaveTextContent( "Product ID", @@ -400,8 +413,8 @@ describe("Notebook Editor > Join Step", () => { await userEvent.click( within(screen.getByLabelText("Right table")).getByRole("button"), ); - const tablePicker = await screen.findByTestId("popover"); - await userEvent.click(await within(tablePicker).findByText("Reviews")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Reviews")); expect(screen.queryByLabelText("Remove condition")).not.toBeInTheDocument(); }); @@ -486,8 +499,8 @@ describe("Notebook Editor > Join Step", () => { it("should be 'all' by default", async () => { const { getRecentJoin } = setup(); - const popover = screen.getByTestId("popover"); - await userEvent.click(await within(popover).findByText("Products")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Products")); await waitFor(() => { const { fields } = getRecentJoin(); @@ -498,8 +511,8 @@ describe("Notebook Editor > Join Step", () => { it("should select a few columns when adding a join", async () => { const { getRecentJoin } = setup(); - const popover = screen.getByTestId("popover"); - await userEvent.click(await within(popover).findByText("Reviews")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Reviews")); await userEvent.click(await screen.findByLabelText("Pick columns")); const joinColumnsPicker = await screen.findByTestId( @@ -546,8 +559,8 @@ describe("Notebook Editor > Join Step", () => { it("should be able to select no columns when adding a new join", async () => { const { getRecentJoin } = setup(); - const popover = screen.getByTestId("popover"); - await userEvent.click(await within(popover).findByText("Reviews")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Reviews")); await userEvent.click(await screen.findByLabelText("Pick columns")); const joinColumnsPicker = await screen.findByTestId( @@ -664,8 +677,8 @@ describe("Notebook Editor > Join Step", () => { await userEvent.click( within(screen.getByLabelText("Right table")).getByRole("button"), ); - const popover = await screen.findByTestId("popover"); - await userEvent.click(await within(popover).findByText("Reviews")); + const modal = await screen.findByTestId("entity-picker-modal"); + await userEvent.click(await within(modal).findByText("Reviews")); expect(screen.queryByLabelText("Add condition")).not.toBeInTheDocument(); diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinTablePicker/JoinTablePicker.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinTablePicker/JoinTablePicker.tsx index 79af4dc5519f9..96e971b78036f 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinTablePicker/JoinTablePicker.tsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinTablePicker/JoinTablePicker.tsx @@ -1,15 +1,18 @@ import type { ReactNode } from "react"; import { useMemo, useState } from "react"; +import { useLatest } from "react-use"; import { t } from "ttag"; -import { DATA_BUCKET } from "metabase/containers/DataPicker/constants"; +import { + DataPickerModal, + dataPickerValueFromJoinable, +} from "metabase/common/components/DataPicker"; +import Questions from "metabase/entities/questions"; import Tables from "metabase/entities/tables"; -import { useDispatch, useSelector } from "metabase/lib/redux"; -import { DataSourceSelector } from "metabase/query_builder/components/DataSelector"; -import { getMetadata } from "metabase/selectors/metadata"; +import { useDispatch } from "metabase/lib/redux"; import { Icon, Popover, Tooltip } from "metabase/ui"; import * as Lib from "metabase-lib"; -import type Table from "metabase-lib/v1/metadata/Table"; +import { getQuestionIdFromVirtualTableId } from "metabase-lib/v1/metadata/utils/saved-questions"; import type { TableId } from "metabase-types/api"; import { NotebookCellItem } from "../../../NotebookCell"; @@ -21,58 +24,65 @@ import { interface JoinTablePickerProps { query: Lib.Query; + stageIndex: number; table: Lib.Joinable | undefined; tableName: string | undefined; color: string; isReadOnly: boolean; - isModelDataSource: boolean; columnPicker: ReactNode; onChange?: (table: Lib.Joinable) => void; } export function JoinTablePicker({ query, - table, + stageIndex, + table: joinable, tableName, color, isReadOnly, - isModelDataSource, columnPicker, onChange, }: JoinTablePickerProps) { - const metadata = useSelector(getMetadata); const dispatch = useDispatch(); + const onChangeRef = useLatest(onChange); + const queryRef = useLatest(query); - const databaseId = useMemo(() => { - return Lib.databaseID(query); - }, [query]); - - const databases = useMemo(() => { - const database = metadata.database(databaseId); - return [database, metadata.savedQuestionsDatabase()].filter(Boolean); - }, [databaseId, metadata]); - - const pickerInfo = useMemo(() => { - return table ? Lib.pickerInfo(query, table) : null; - }, [query, table]); + const [isDataPickerOpen, setIsDataPickerOpen] = useState(!joinable); + const databaseId = useMemo(() => Lib.databaseID(query), [query]); - const tableId = pickerInfo?.tableId ?? pickerInfo?.cardId; - const tableFilter = (table: Table) => !tableId || table.db_id === databaseId; const isDisabled = isReadOnly; const handleTableChange = async (tableId: TableId) => { + // we need to populate query metadata with selected table await dispatch(Tables.actions.fetchMetadata({ id: tableId })); - onChange?.(Lib.tableOrCardMetadata(query, tableId)); + + if (typeof tableId === "string") { + await dispatch( + Questions.actions.fetch({ + id: getQuestionIdFromVirtualTableId(tableId), + }), + ); + } + + onChangeRef.current?.(Lib.tableOrCardMetadata(queryRef.current, tableId)); }; + const value = useMemo(() => { + if (!joinable) { + return undefined; + } + + return dataPickerValueFromJoinable(query, stageIndex, joinable); + }, [query, stageIndex, joinable]); + return ( ) : null } @@ -80,25 +90,22 @@ export function JoinTablePicker({ rightContainerStyle={RIGHT_CONTAINER_STYLE} aria-label={t`Right table`} > - - {tableName || t`Pick data…`} - - } - /> + setIsDataPickerOpen(true)} + > + {tableName || t`Pick data…`} + + + {isDataPickerOpen && ( + setIsDataPickerOpen(false)} + /> + )} ); } @@ -137,16 +144,3 @@ const RIGHT_CONTAINER_STYLE = { height: 37, padding: 0, }; - -function getSelectedDataBucketId( - pickerInfo: Lib.PickerInfo | null, - isModelDataSource: boolean, -) { - if (pickerInfo?.tableId != null) { - return undefined; - } - if (isModelDataSource) { - return DATA_BUCKET.MODELS; - } - return undefined; -} diff --git a/frontend/src/metabase/query_builder/components/notebook/types.ts b/frontend/src/metabase/query_builder/components/notebook/types.ts index 316fed97fed95..a1be8af277901 100644 --- a/frontend/src/metabase/query_builder/components/notebook/types.ts +++ b/frontend/src/metabase/query_builder/components/notebook/types.ts @@ -41,7 +41,6 @@ export interface NotebookStepUiComponentProps { step: NotebookStep; query: Query; stageIndex: number; - sourceQuestion?: Question; color: string; isLastOpened: boolean; reportTimezone: string; diff --git a/frontend/src/metabase/query_builder/components/view/QuestionDataSelector/QuestionDataSelector.tsx b/frontend/src/metabase/query_builder/components/view/QuestionDataSelector/QuestionDataSelector.tsx deleted file mode 100644 index 2bf4991e05cbe..0000000000000 --- a/frontend/src/metabase/query_builder/components/view/QuestionDataSelector/QuestionDataSelector.tsx +++ /dev/null @@ -1,35 +0,0 @@ -import CS from "metabase/css/core/index.css"; -import type { updateQuestion as updateQuestionAction } from "metabase/query_builder/actions"; -import { DataSourceSelector } from "metabase/query_builder/components/DataSelector"; -import * as Lib from "metabase-lib"; -import type Question from "metabase-lib/v1/Question"; -import type { DatabaseId, TableId } from "metabase-types/api"; - -export const QuestionDataSelector = ({ - question, - updateQuestion, - triggerElement, -}: { - question: Question; - updateQuestion: typeof updateQuestionAction; - triggerElement: JSX.Element; -}) => { - const handleTableChange = (tableId: TableId, databaseId: DatabaseId) => { - const metadata = question.metadata(); - const metadataProvider = Lib.metadataProvider(databaseId, metadata); - const table = Lib.tableOrCardMetadata(metadataProvider, tableId); - const query = Lib.queryFromTableOrCardMetadata(metadataProvider, table); - updateQuestion(question.setQuery(query), { run: true }); - }; - - return ( - - ); -}; diff --git a/frontend/src/metabase/query_builder/components/view/QuestionDataSelector/index.ts b/frontend/src/metabase/query_builder/components/view/QuestionDataSelector/index.ts deleted file mode 100644 index 72954fbe4de16..0000000000000 --- a/frontend/src/metabase/query_builder/components/view/QuestionDataSelector/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { QuestionDataSelector } from "./QuestionDataSelector"; diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx index f7338f243ec8e..04d5102c061cf 100644 --- a/frontend/src/metabase/query_builder/components/view/View.jsx +++ b/frontend/src/metabase/query_builder/components/view/View.jsx @@ -31,7 +31,6 @@ import { SnippetSidebar } from "../template_tags/SnippetSidebar"; import { TagEditorSidebar } from "../template_tags/TagEditorSidebar"; import NewQuestionHeader from "./NewQuestionHeader"; -import NewQuestionView from "./View/NewQuestionView"; import { NotebookContainer } from "./View/NotebookContainer"; import { BorderedViewTitleHeader, @@ -372,7 +371,6 @@ class View extends Component { onConfirmToast, isShowingToaster, isHeaderVisible, - updateQuestion, } = this.props; // if we don't have a question at all or no databases then we are initializing, so keep it simple @@ -384,17 +382,6 @@ class View extends Component { const { isNative } = Lib.queryDisplayInfo(question.query()); const isNewQuestion = !isNative && Lib.sourceTableOrCardId(query) === null; - - if (isNewQuestion && queryBuilderMode === "view") { - return ( - - ); - } - const isModel = question.type() === "model"; if (isModel && queryBuilderMode === "dataset") { diff --git a/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/NewQuestionView.tsx b/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/NewQuestionView.tsx deleted file mode 100644 index 31f68f691bb58..0000000000000 --- a/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/NewQuestionView.tsx +++ /dev/null @@ -1,32 +0,0 @@ -import cx from "classnames"; -import { t } from "ttag"; - -import Subhead from "metabase/components/type/Subhead"; -import CS from "metabase/css/core/index.css"; -import type { updateQuestion } from "metabase/query_builder/actions"; -import { QuestionDataSelector } from "metabase/query_builder/components/view/QuestionDataSelector"; -import type Question from "metabase-lib/v1/Question"; - -type Props = { - question: Question; - updateQuestion: typeof updateQuestion; -}; - -function NewQuestionView({ question, updateQuestion }: Props) { - return ( -
-
- {t`Pick your data`} - } - /> -
-
- ); -} - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default NewQuestionView; diff --git a/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/index.ts b/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/index.ts deleted file mode 100644 index 3870b6d98ce0c..0000000000000 --- a/frontend/src/metabase/query_builder/components/view/View/NewQuestionView/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -// eslint-disable-next-line import/no-default-export -- deprecated usage -export { default } from "./NewQuestionView"; diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.beforeunload-events.unit.spec.tsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.beforeunload-events.unit.spec.tsx index 38a76d3e3e3ed..d0799d412f1e3 100644 --- a/frontend/src/metabase/query_builder/containers/QueryBuilder.beforeunload-events.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.beforeunload-events.unit.spec.tsx @@ -24,7 +24,21 @@ import { registerVisualizations(); describe("QueryBuilder - beforeunload events", () => { + const scrollBy = HTMLElement.prototype.scrollBy; + const getBoundingClientRect = HTMLElement.prototype.getBoundingClientRect; + + beforeEach(() => { + HTMLElement.prototype.scrollBy = jest.fn(); + // needed for @tanstack/react-virtual, see https://github.com/TanStack/virtual/issues/29#issuecomment-657519522 + HTMLElement.prototype.getBoundingClientRect = jest + .fn() + .mockReturnValue({ height: 1, width: 1 }); + }); + afterEach(() => { + HTMLElement.prototype.scrollBy = scrollBy; + HTMLElement.prototype.getBoundingClientRect = getBoundingClientRect; + jest.resetAllMocks(); }); diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.unsaved-changes-warning.unit.spec.tsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.unsaved-changes-warning.unit.spec.tsx index 282bfd3466006..955f2808e32de 100644 --- a/frontend/src/metabase/query_builder/containers/QueryBuilder.unsaved-changes-warning.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.unsaved-changes-warning.unit.spec.tsx @@ -39,7 +39,21 @@ import { registerVisualizations(); describe("QueryBuilder - unsaved changes warning", () => { + const scrollBy = HTMLElement.prototype.scrollBy; + const getBoundingClientRect = HTMLElement.prototype.getBoundingClientRect; + + beforeEach(() => { + HTMLElement.prototype.scrollBy = jest.fn(); + // needed for @tanstack/react-virtual, see https://github.com/TanStack/virtual/issues/29#issuecomment-657519522 + HTMLElement.prototype.getBoundingClientRect = jest + .fn() + .mockReturnValue({ height: 1, width: 1 }); + }); + afterEach(() => { + HTMLElement.prototype.scrollBy = scrollBy; + HTMLElement.prototype.getBoundingClientRect = getBoundingClientRect; + jest.resetAllMocks(); }); @@ -217,9 +231,12 @@ describe("QueryBuilder - unsaved changes warning", () => { it("does not show custom warning modal when saving edited query", async () => { const { history } = await setup({ card: TEST_MODEL_CARD, - initialRoute: `/model/${TEST_MODEL_CARD.id}/query`, + initialRoute: "/", }); + history.push(`/model/${TEST_MODEL_CARD.id}/query`); + await waitForLoaderToBeRemoved(); + await triggerNotebookQueryChange(); await waitForSaveChangesToBeEnabled(); @@ -310,9 +327,12 @@ describe("QueryBuilder - unsaved changes warning", () => { const { history } = await setup({ card: TEST_MODEL_CARD, dataset: TEST_MODEL_DATASET, - initialRoute: `/model/${TEST_MODEL_CARD.id}/query`, + initialRoute: "/", }); + history.push(`/model/${TEST_MODEL_CARD.id}/query`); + await waitForLoaderToBeRemoved(); + /** * When initialRoute is `/model/${TEST_MODEL_CARD.id}/metadata`, * the QueryBuilder gets incompletely intialized. @@ -718,9 +738,12 @@ describe("QueryBuilder - unsaved changes warning", () => { it("does not show custom warning modal when saving edited question", async () => { const { history } = await setup({ card: TEST_STRUCTURED_CARD, - initialRoute: `/question/${TEST_STRUCTURED_CARD.id}/notebook`, + initialRoute: "/", }); + history.push(`/question/${TEST_STRUCTURED_CARD.id}/notebook`); + await waitForLoaderToBeRemoved(); + await triggerNotebookQueryChange(); await waitForSaveToBeEnabled(); diff --git a/frontend/src/metabase/query_builder/containers/test-utils.tsx b/frontend/src/metabase/query_builder/containers/test-utils.tsx index f226e8b92c689..e8a736bf87267 100644 --- a/frontend/src/metabase/query_builder/containers/test-utils.tsx +++ b/frontend/src/metabase/query_builder/containers/test-utils.tsx @@ -17,6 +17,7 @@ import { setupSearchEndpoints, setupTimelinesEndpoints, setupPropertiesEndpoints, + setupRecentViewsEndpoints, } from "__support__/server-mocks"; import { renderWithProviders, @@ -241,6 +242,7 @@ export const setup = async ({ setupFieldValuesEndpoints( createMockFieldValues({ field_id: Number(ORDERS.QUANTITY) }), ); + setupRecentViewsEndpoints([]); if (isSavedCard(card)) { setupCardsEndpoints([card]); @@ -319,11 +321,9 @@ export const startNewNotebookModel = async () => { await userEvent.click(screen.getByText("Use the notebook editor")); await waitForLoaderToBeRemoved(); - await userEvent.click(screen.getByText("Pick your starting data")); - const popover = screen.getByTestId("popover"); - await userEvent.click(within(popover).getByText("Sample Database")); + const modal = await screen.findByTestId("entity-picker-modal"); await waitForLoaderToBeRemoved(); - await userEvent.click(within(popover).getByText("Orders")); + await userEvent.click(within(modal).getByText("Orders")); expect(screen.getByRole("button", { name: "Get Answer" })).toBeEnabled(); }; diff --git a/src/metabase/lib/card.cljc b/src/metabase/lib/card.cljc index 896b8861b0fbe..66abe5eab6758 100644 --- a/src/metabase/lib/card.cljc +++ b/src/metabase/lib/card.cljc @@ -21,6 +21,11 @@ (cond-> card-metadata (not display-name) (assoc :display-name (u.humanization/name->human-readable-name :simple card-name)))) +(defmethod lib.metadata.calculation/display-info-method :metadata/card + [query stage-number card-metadata] + (cond-> ((get-method lib.metadata.calculation/display-info-method :default) query stage-number card-metadata) + (= (:type card-metadata) :model) (assoc :model? true))) + (defmethod lib.metadata.calculation/visible-columns-method :metadata/card [query stage-number diff --git a/src/metabase/lib/metadata/calculation.cljc b/src/metabase/lib/metadata/calculation.cljc index b0cbb2e824284..d39c4cc249824 100644 --- a/src/metabase/lib/metadata/calculation.cljc +++ b/src/metabase/lib/metadata/calculation.cljc @@ -396,7 +396,8 @@ (defmethod display-info-method :metadata/table [query stage-number table] (merge (default-display-info query stage-number table) - {:is-source-table (= (lib.util/source-table-id query) (:id table))})) + {:is-source-table (= (lib.util/source-table-id query) (:id table)) + :schema (:schema table)})) (def ColumnMetadataWithSource "Schema for the column metadata that should be returned by [[metadata]]."