Skip to content
Snippets Groups Projects
Unverified Commit e122df47 authored by Nemanja Glumac's avatar Nemanja Glumac Committed by GitHub
Browse files

MS1: Open notebook data source in new tab (#46228)

* WIP Open notebook data source in new tab

* WIP

* Implement the new functionality with `pickerInfo`

* Use destructured value

* Add the simplest possible E2E test as PoC

* Smoke test

* Add basic coverage for questions

* Add basic coverage for models

* Source question has been archived scenario

* Add basic permissions test

* Expand to the user without wite query permissions

* Fix title

* Add another permission scenario

* Add E2E coverage for a source being native

* Add coverage for the native model

* Address review comment re: `pickerInfo`

* Extract `getUrl` to utils

* Add support for the middle/scroll click

* Add unit tests

* Add a negative test case

* Fix title
parent beda4b0c
No related branches found
No related tags found
No related merge requests found
import {
ADMIN_PERSONAL_COLLECTION_ID,
ORDERS_COUNT_QUESTION_ID,
ORDERS_MODEL_ID,
} from "e2e/support/cypress_sample_instance_data";
import {
createNativeQuestion,
createQuestion,
getNotebookStep,
openNotebook,
openReviewsTable,
popover,
restore,
tableInteractive,
visitModel,
visitQuestion,
visualize,
type NativeQuestionDetails,
} from "e2e/support/helpers";
// https://docs.cypress.io/api/cypress-api/platform
const macOSX = Cypress.platform === "darwin";
const clickConfig = {
metaKey: macOSX,
ctrlKey: !macOSX,
};
describe("scenarios > notebook > link to data source", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
cy.on("window:before:load", win => {
// prevent Cypress opening in a new window/tab and spy on this method
cy.stub(win, "open").callsFake(url => {
expect(win.open).to.be.calledOnce;
// replace the current page with the linked data source upon ctrl/meta click
win.location.replace(url);
});
});
});
it("smoke test", () => {
openReviewsTable({ mode: "notebook" });
cy.log("Normal click on the data source still opens the entity picker");
getNotebookStep("data").findByText("Reviews").click();
cy.findByTestId("entity-picker-modal").within(() => {
cy.findByText("Pick your starting data").should("be.visible");
cy.findByLabelText("Close").click();
});
cy.log("Meta/Ctrl click on the fields picker behaves as a regular click");
getNotebookStep("data").findByTestId("fields-picker").click(clickConfig);
popover().within(() => {
cy.findByText("Select none").click();
});
// Regular click on the fields picker again to close the popover
getNotebookStep("data").findByTestId("fields-picker").click();
// Mid-test sanity-check assertion
visualize();
cy.findAllByTestId("header-cell")
.should("have.length", 1)
.and("have.text", "ID");
cy.log(
"Deselecting columns should have no effect on the linked data source in new tab/window",
);
openNotebook();
getNotebookStep("data").findByText("Reviews").click(clickConfig);
cy.wait("@dataset"); // already intercepted in `visualize()`
cy.log("Make sure Reviews table is rendered in a simple mode");
cy.findAllByTestId("header-cell").should("contain", "Reviewer");
tableInteractive().should("contain", "xavier");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing 1,112 rows",
);
cy.findByTestId("qb-save-button").should("be.enabled");
});
context("questions", () => {
it("should open the source table from a simple question", () => {
visitQuestion(ORDERS_COUNT_QUESTION_ID);
openNotebook();
getNotebookStep("data").findByText("Orders").click(clickConfig);
cy.log("Make sure Orders table is rendered in a simple mode");
cy.findAllByTestId("header-cell").should("contain", "Subtotal");
tableInteractive().should("contain", "37.65");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing first 2,000 rows",
);
cy.findByTestId("qb-save-button").should("be.enabled");
});
it("should open the source question from a nested question", () => {
createQuestion(
{
name: "Nested question based on a question",
query: { "source-table": `card__${ORDERS_COUNT_QUESTION_ID}` },
},
{ visitQuestion: true },
);
openNotebook();
getNotebookStep("data").findByText("Orders, Count").click(clickConfig);
cy.log("Make sure the source question rendered in a simple mode");
cy.location("pathname").should(
"eq",
`/question/${ORDERS_COUNT_QUESTION_ID}-orders-count`,
);
cy.findAllByTestId("header-cell")
.should("have.length", "1")
.and("have.text", "Count");
tableInteractive().should("contain", "18,760");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing 1 row",
);
// Question is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
it("should open the source question from a nested question where the source is native question", () => {
const source = {
name: "Native source",
native: {
query: "select 1 as foo",
"template-tags": {},
},
};
createNativeQuestion(source).then(({ body: sourceQuestion }) => {
createQuestion(
{
name: "Nested question based on a native question",
query: { "source-table": `card__${sourceQuestion.id}` },
},
{ visitQuestion: true },
);
openNotebook();
getNotebookStep("data").findByText(source.name).click(clickConfig);
cy.log("Make sure the source question rendered in a simple mode");
cy.location("pathname").should(
"eq",
`/question/${sourceQuestion.id}-native-source`,
);
cy.findAllByTestId("header-cell")
.should("have.length", "1")
.and("have.text", "FOO");
cy.get("#main-data-grid")
.findByTestId("cell-data")
.should("have.text", "1");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing 1 row",
);
// Question is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
});
it("should open the source model from a nested question", () => {
createQuestion(
{
name: "Nested question based on a model",
query: { "source-table": `card__${ORDERS_MODEL_ID}` },
},
{ visitQuestion: true },
);
openNotebook();
getNotebookStep("data").findByText("Orders Model").click(clickConfig);
cy.log("Make sure the source model is rendered in a simple mode");
cy.location("pathname").should(
"eq",
`/model/${ORDERS_MODEL_ID}-orders-model`,
);
cy.findAllByTestId("header-cell").should("contain", "Subtotal");
tableInteractive().should("contain", "37.65");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing first 2,000 rows",
);
// Model is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
it("should open the source model from a nested question where the source is native model", () => {
const source: NativeQuestionDetails = {
name: "Native source",
native: {
query: "select 1 as foo",
"template-tags": {},
},
type: "model",
};
createNativeQuestion(source).then(({ body: sourceQuestion }) => {
createQuestion(
{
name: "Nested question based on a native question",
query: { "source-table": `card__${sourceQuestion.id}` },
},
{ visitQuestion: true },
);
openNotebook();
getNotebookStep("data")
.findByText(sourceQuestion.name)
.click(clickConfig);
cy.log("Make sure the source model rendered in a simple mode");
cy.location("pathname").should(
"eq",
`/model/${sourceQuestion.id}-native-source`,
);
cy.findByTestId("scalar-value").should("have.text", "1");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing 1 row",
);
// Model is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
});
it('should open the "trash" if the source question has been archived', () => {
createQuestion({
name: "Nested question based on a question",
query: { "source-table": `card__${ORDERS_COUNT_QUESTION_ID}` },
}).then(({ body: nestedQuestion }) => {
cy.log("Move the source question to the trash");
cy.request("PUT", `/api/card/${ORDERS_COUNT_QUESTION_ID}`, {
archived: true,
});
visitQuestion(nestedQuestion.id);
});
openNotebook();
getNotebookStep("data").findByText("Orders, Count").click(clickConfig);
cy.log('Make sure the source question opens in the "trash"');
cy.location("pathname").should(
"eq",
`/question/${ORDERS_COUNT_QUESTION_ID}-orders-count`,
);
cy.findByTestId("archive-banner").should(
"contain",
"This question is in the trash",
);
});
});
context("models", () => {
it("should open the underlying model", () => {
visitModel(ORDERS_MODEL_ID);
openNotebook();
getNotebookStep("data").findByText("Orders Model").click(clickConfig);
cy.log("Make sure the source model is rendered in a simple mode");
cy.location("pathname").should(
"eq",
`/model/${ORDERS_MODEL_ID}-orders-model`,
);
cy.findAllByTestId("header-cell").should("contain", "Subtotal");
tableInteractive().should("contain", "37.65");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing first 2,000 rows",
);
// Model is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
it("should open the underlying native model", () => {
const model: NativeQuestionDetails = {
name: "Native model",
native: {
query: "select 1 as foo",
"template-tags": {},
},
type: "model",
};
createNativeQuestion(model).then(({ body: { id, name } }) => {
visitModel(id);
openNotebook();
getNotebookStep("data").findByText(name).click(clickConfig);
cy.log("Make sure the source model rendered in a simple mode");
cy.location("pathname").should("eq", `/model/${id}-native-model`);
cy.findByTestId("scalar-value").should("have.text", "1");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing 1 row",
);
// Model is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
});
it("should open the nested model (based on a question) as the data source", () => {
createQuestion(
{
name: "Nested model based on a question",
query: { "source-table": `card__${ORDERS_COUNT_QUESTION_ID}` },
type: "model",
},
{ visitQuestion: true, wrapId: true, idAlias: "nestedModelId" },
);
openNotebook();
getNotebookStep("data")
.findByText("Nested model based on a question")
.click(clickConfig);
cy.log("Make sure the source model is rendered in a simple mode");
cy.get("@nestedModelId").then(id => {
cy.location("pathname").should(
"eq",
`/model/${id}-nested-model-based-on-a-question`,
);
cy.findByTestId("scalar-value").should("have.text", "18,760");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing 1 row",
);
// Model is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
});
it("should open the nested model (based on a model) as the data source", () => {
createQuestion(
{
name: "Nested model based on a model",
query: { "source-table": `card__${ORDERS_MODEL_ID}` },
type: "model",
},
{ visitQuestion: true, wrapId: true, idAlias: "nestedModelId" },
);
openNotebook();
getNotebookStep("data")
.findByText("Nested model based on a model")
.click(clickConfig);
cy.log("Make sure the source model is rendered in a simple mode");
cy.get("@nestedModelId").then(id => {
cy.location("pathname").should(
"eq",
`/model/${id}-nested-model-based-on-a-model`,
);
cy.findAllByTestId("header-cell").should("contain", "Subtotal");
tableInteractive().should("contain", "37.65");
cy.findByTestId("question-row-count").should(
"have.text",
"Showing first 2,000 rows",
);
// Model is not dirty
cy.findByTestId("qb-save-button").should("not.exist");
});
});
});
context("permissions", () => {
it("shouldn't show the source question if it lives in a collection that user can't see", () => {
createQuestion({
name: "Nested question based on a question",
query: { "source-table": `card__${ORDERS_COUNT_QUESTION_ID}` },
}).then(({ body: nestedQuestion }) => {
cy.log("Move the source question to admin's personal collection");
cy.request("PUT", `/api/card/${ORDERS_COUNT_QUESTION_ID}`, {
collection_id: ADMIN_PERSONAL_COLLECTION_ID,
});
cy.signInAsNormalUser();
visitQuestion(nestedQuestion.id);
cy.log("We should not even show the notebook icon");
cy.findByTestId("qb-header-action-panel")
.icon("notebook")
.should("not.exist");
cy.log(
"Even if user opens the notebook link directly, they should not see the source question. We open the entity picker instead",
);
cy.visit(`/question/${nestedQuestion.id}/notebook`);
cy.findByTestId("entity-picker-modal").within(() => {
cy.findByText("Pick your starting data").should("be.visible");
cy.findByLabelText("Close").click();
});
getNotebookStep("data").should("contain", "Pick your starting data");
cy.log(
"The same should be true for a user that additionally doesn't have write query permissions",
);
cy.signIn("nodata");
visitQuestion(nestedQuestion.id);
cy.findByTestId("qb-header-action-panel")
.icon("notebook")
.should("not.exist");
cy.visit(`/question/${nestedQuestion.id}/notebook`);
cy.findByTestId("entity-picker-modal").within(() => {
cy.findByText("Pick your starting data").should("be.visible");
cy.findByLabelText("Close").click();
});
getNotebookStep("data").should("contain", "Pick your starting data");
});
});
it("user with the curate collection permissions but without write query permissions shouldn't be able to see/open the source question", () => {
createQuestion({
name: "Nested question based on a question",
query: { "source-table": `card__${ORDERS_COUNT_QUESTION_ID}` },
}).then(({ body: nestedQuestion }) => {
cy.signIn("nodata");
visitQuestion(nestedQuestion.id);
cy.log("We should not even show the notebook icon");
cy.findByTestId("qb-header-action-panel")
.icon("notebook")
.should("not.exist");
// TODO update the following once metabase##46398 is fixed
// cy.visit(`/question/${nestedQuestion.id}/notebook`);
});
});
});
});
import { useMemo, useState } from "react";
import { useMemo, useState, type MouseEvent } from "react";
import { useLatest } from "react-use";
import {
......@@ -16,6 +16,8 @@ import type { DatabaseId, TableId } from "metabase-types/api";
import { NotebookCell } from "../NotebookCell";
import { getUrl, openInNewTab } from "./utils";
interface NotebookDataPickerProps {
title: string;
query: Lib.Query;
......@@ -66,6 +68,31 @@ export function NotebookDataPicker({
onChangeRef.current?.(table, metadataProvider);
};
const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
const isCtrlOrMetaClick =
(event.ctrlKey || event.metaKey) && event.button === 0;
if (isCtrlOrMetaClick) {
const url = getUrl({ query, table, stageIndex });
openInNewTab(url);
} else {
setIsOpen(true);
}
};
const handleAuxClick = (event: MouseEvent<HTMLButtonElement>) => {
const isMiddleClick = event.button === 1;
if (isMiddleClick) {
const url = getUrl({ query, table, stageIndex });
openInNewTab(url);
} else {
setIsOpen(true);
}
};
return (
<>
<UnstyledButton
......@@ -74,7 +101,8 @@ export function NotebookDataPicker({
fw="inherit"
p={NotebookCell.CONTAINER_PADDING}
disabled={isDisabled}
onClick={() => setIsOpen(true)}
onClick={handleClick}
onAuxClick={handleAuxClick}
>
<Group spacing="xs">
{tableInfo && <Icon name={getTableIcon(tableInfo)} />}
......
import * as Urls from "metabase/lib/urls";
import * as Lib from "metabase-lib";
type Props = {
query: Lib.Query;
table?: Lib.TableMetadata | Lib.CardMetadata;
stageIndex: number;
};
export const getUrl = ({
query,
table,
stageIndex,
}: Props): string | undefined => {
if (!table) {
return;
}
const pickerInfo = Lib.pickerInfo(query, table);
const tableInfo = Lib.displayInfo(query, stageIndex, table);
if (!pickerInfo || !tableInfo) {
return;
}
const { isModel, cardId, tableId, databaseId } = pickerInfo;
if (cardId) {
const payload = {
id: cardId,
name: tableInfo.displayName,
};
return isModel ? Urls.model(payload) : Urls.question(payload);
} else {
return Urls.tableRowsQuery(databaseId, tableId);
}
};
export const openInNewTab = (link?: string) => {
if (!link) {
return;
}
window.open(link, "_blank");
};
import { fireEvent } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { createMockMetadata } from "__support__/metadata";
......@@ -52,6 +53,8 @@ const setup = (
step = createMockNotebookStep(),
{ readOnly = false }: { readOnly?: boolean } = {},
) => {
const mockWindowOpen = jest.spyOn(window, "open").mockImplementation();
const updateQuery = jest.fn();
setupDatabasesEndpoints([createSampleDatabase()]);
setupSearchEndpoints([]);
......@@ -89,7 +92,7 @@ const setup = (
return Lib.displayInfo(nextQuery, 0, column);
};
return { getNextQuery, getNextTableName, getNextColumn };
return { getNextQuery, getNextTableName, getNextColumn, mockWindowOpen };
};
const setupEmptyQuery = () => {
......@@ -290,4 +293,54 @@ describe("DataStep", () => {
expect(screen.queryByLabelText("Pick columns")).not.toBeInTheDocument();
});
});
describe("link to data source", () => {
it("meta click should open the data source in a new window", () => {
const { mockWindowOpen } = setup();
const dataSource = screen.getByText("Orders");
fireEvent.click(dataSource, { metaKey: true });
expect(mockWindowOpen).toHaveBeenCalledTimes(1);
mockWindowOpen.mockClear();
});
it("ctrl click should open the data source in a new window", () => {
const { mockWindowOpen } = setup();
const dataSource = screen.getByText("Orders");
fireEvent.click(dataSource, { ctrlKey: true });
expect(mockWindowOpen).toHaveBeenCalledTimes(1);
mockWindowOpen.mockClear();
});
it("middle click should open the data source in a new window", () => {
const { mockWindowOpen } = setup();
const dataSource = screen.getByText("Orders");
const middleClick = new MouseEvent("auxclick", {
bubbles: true,
button: 1,
});
fireEvent(dataSource, middleClick);
expect(mockWindowOpen).toHaveBeenCalledTimes(1);
mockWindowOpen.mockClear();
});
it("regular click should open the entity picker", async () => {
const { mockWindowOpen } = setup();
const dataSource = screen.getByText("Orders");
fireEvent.click(dataSource);
expect(
await screen.findByTestId("entity-picker-modal"),
).toBeInTheDocument();
expect(mockWindowOpen).not.toHaveBeenCalled();
});
});
});
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment