diff --git a/frontend/src/metabase/query_builder/components/DataSelector.jsx b/frontend/src/metabase/query_builder/components/DataSelector.jsx
index 423fbdb66a1b63fc120bdb8c57fe53c603a26683..6377b2454268543c6cf46b2b0c13af9979a0e1c3 100644
--- a/frontend/src/metabase/query_builder/components/DataSelector.jsx
+++ b/frontend/src/metabase/query_builder/components/DataSelector.jsx
@@ -1009,7 +1009,10 @@ const TablePicker = ({
},
];
return (
- <div style={{ width: 300, overflowY: "auto" }}>
+ <div
+ style={{ width: 300, overflowY: "auto" }}
+ data-testid="data-selector"
+ >
<AccordionList
id="TablePicker"
key="tablePicker"
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookCell.jsx b/frontend/src/metabase/query_builder/components/notebook/NotebookCell.jsx
index c006ae0f534ab55bbf4d094375541b9dd6489224..8b1e20f8e99be0da13de30a2927e17ea7655f519 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookCell.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookCell.jsx
@@ -2,7 +2,7 @@
import React from "react";
import { Flex } from "grid-styled";
-import styled from "styled-components";
+import styled, { css } from "styled-components";
import Icon from "metabase/components/Icon";
@@ -14,45 +14,101 @@ export const NotebookCell = styled(Flex).attrs({
})`
border-radius: 8px;
background-color: ${props => alpha(props.color, 0.1)};
+ padding: 14px;
`;
-NotebookCell.defaultProps = {
- px: 2,
- pt: 2,
- pb: 1,
-};
-
NotebookCell.displayName = "NotebookCell";
-export const NotebookCellItem = styled(Flex).attrs({
- align: "center",
- children: ({ icon, children }) => [
- icon && <Icon className="mr1" name={icon} size={10} />,
- children,
- ],
-})`
+const NotebookCellItemContainer = styled(Flex).attrs({ align: "center" })`
font-weight: bold;
- border: 2px solid transparent;
- border-radius: 6px;
color: ${props => (props.inactive ? props.color : "white")};
- background-color: ${props => (props.inactive ? "transparent" : props.color)};
+ border-radius: 6px;
+ margin-right: 4px;
+
+ border: 2px solid transparent;
border-color: ${props =>
props.inactive ? alpha(props.color, 0.25) : "transparent"};
+
&:hover {
- background-color: ${props => !props.inactive && alpha(props.color, 0.8)};
border-color: ${props => props.inactive && alpha(props.color, 0.8)};
}
- transition: background 300ms linear, border 300ms linear;
- > .Icon {
+
+ transition: border 300ms linear;
+
+ .Icon-close {
opacity: 0.6;
}
`;
-NotebookCellItem.defaultProps = {
- p: 1,
- mr: 1,
- mb: 1,
-};
+const NotebookCellItemContentContainer = styled.div`
+ display: flex;
+ align-items: center;
+ padding: 10px;
+ background-color: ${props => (props.inactive ? "transparent" : props.color)};
+
+ &:hover {
+ background-color: ${props => !props.inactive && alpha(props.color, 0.8)};
+ }
+
+ ${props =>
+ !!props.border &&
+ css`
+ border-${props.border}: 1px solid ${alpha("white", 0.25)};
+ `}
+
+ ${props =>
+ props.roundedCorners.includes("left") &&
+ css`
+ border-top-left-radius: 6px;
+ border-bottom-left-radius: 6px;
+ `}
+
+ ${props =>
+ props.roundedCorners.includes("right") &&
+ css`
+ border-top-right-radius: 6px;
+ border-bottom-right-radius: 6px;
+ `}
+
+ transition: background 300ms linear;
+`;
+
+export function NotebookCellItem({
+ inactive,
+ color,
+ right,
+ rightContainerStyle,
+ children,
+ ...props
+}) {
+ const hasRightSide = React.isValidElement(right);
+ const mainContentRoundedCorners = ["left"];
+ if (!hasRightSide) {
+ mainContentRoundedCorners.push("right");
+ }
+ return (
+ <NotebookCellItemContainer inactive={inactive} color={color} {...props}>
+ <NotebookCellItemContentContainer
+ inactive={inactive}
+ color={color}
+ roundedCorners={mainContentRoundedCorners}
+ >
+ {children}
+ </NotebookCellItemContentContainer>
+ {hasRightSide && (
+ <NotebookCellItemContentContainer
+ inactive={inactive}
+ color={color}
+ border="left"
+ roundedCorners={["right"]}
+ style={rightContainerStyle}
+ >
+ {right}
+ </NotebookCellItemContentContainer>
+ )}
+ </NotebookCellItemContainer>
+ );
+}
NotebookCellItem.displayName = "NotebookCellItem";
@@ -61,14 +117,6 @@ export const NotebookCellAdd = styled(NotebookCellItem).attrs({
// eslint-disable-next-line react/display-name
children: ({ initialAddText }) =>
initialAddText || <Icon name="add" className="text-white" />,
-})`
- > .Icon {
- opacity: 1;
- }
-`;
-
-NotebookCellAdd.defaultProps = {
- mb: 1,
-};
+})``;
NotebookCellAdd.displayName = "NotebookCellAdd";
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx
index 1138332df0228e199fcc1ea3905a6e729d496777..4364d7672da71330a37b384c9e6c093b32260380 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/DataStep.jsx
@@ -30,11 +30,7 @@ function DataStep({ color, query, databases, updateQuery }) {
{t`Pick your starting data`}
</NotebookCellItem>
) : (
- <NotebookCellItem
- color={color}
- icon="table2"
- data-testid="data-step-cell"
- >
+ <NotebookCellItem color={color} data-testid="data-step-cell">
{table && table.displayName()}
</NotebookCellItem>
)
@@ -42,7 +38,7 @@ function DataStep({ color, query, databases, updateQuery }) {
/>
{table && query.isRaw() && (
<DataFieldsPicker
- className="ml-auto mb1 text-bold"
+ className="ml-auto text-bold"
query={query}
updateQuery={updateQuery}
/>
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx
index 820f5a0d72f0cdd2240becb30bedecc6ead0e142..333336199c818e45a20343bb7df1b3429624bc01 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.jsx
@@ -168,7 +168,7 @@ function JoinClause({ color, join, updateQuery, showRemove }) {
return (
<JoinClauseRoot>
- <NotebookCellItem color={color} icon="table2">
+ <NotebookCellItem color={color}>
{(lhsTable && lhsTable.displayName()) || `Previous results`}
</NotebookCellItem>
@@ -194,6 +194,7 @@ function JoinClause({ color, join, updateQuery, showRemove }) {
options={join.parentDimensionOptions()}
onChange={onParentDimensionChange}
ref={parentDimensionPickerRef}
+ data-testid="parent-dimension"
/>
<JoinOnConditionLabel />
@@ -205,13 +206,14 @@ function JoinClause({ color, join, updateQuery, showRemove }) {
options={join.joinDimensionOptions()}
onChange={onJoinDimensionChange}
ref={joinDimensionPickerRef}
+ data-testid="join-dimension"
/>
</JoinedTableControlRoot>
)}
{join.isValid() && (
<JoinFieldsPicker
- className="mb1 ml-auto text-bold"
+ className="ml-auto text-bold"
join={join}
updateQuery={updateQuery}
/>
@@ -256,7 +258,7 @@ function JoinTablePicker({
}
return (
- <NotebookCellItem color={color} icon="table2" inactive={!joinedTable}>
+ <NotebookCellItem color={color} inactive={!joinedTable}>
<DatabaseSchemaAndTableDataSelector
hasTableSearch
canChangeDatabase={false}
@@ -380,6 +382,7 @@ const joinDimensionPickerPropTypes = {
}).isRequired,
query: PropTypes.object.isRequired,
color: PropTypes.string,
+ "data-testid": PropTypes.string,
};
class JoinDimensionPicker extends React.Component {
@@ -388,14 +391,15 @@ class JoinDimensionPicker extends React.Component {
}
render() {
const { dimension, onChange, options, query, color } = this.props;
+ const testID = this.props["data-testid"] || "join-dimension";
return (
<PopoverWithTrigger
ref={ref => (this._popover = ref)}
triggerElement={
<NotebookCellItem
color={color}
- icon={dimension && dimension.icon()}
inactive={!dimension}
+ data-testid={testID}
>
{dimension ? dimension.displayName() : `Pick a column...`}
</NotebookCellItem>
@@ -412,6 +416,7 @@ class JoinDimensionPicker extends React.Component {
onChange(field);
onClose();
}}
+ data-testid={`${testID}-picker`}
/>
)}
</PopoverWithTrigger>
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.styled.js b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.styled.js
index b6d757e04a288a078fea64bc5a3a93fb8ae261f9..4d89c801226faba829e0533d00e452d98e24dab0 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.styled.js
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.styled.js
@@ -21,7 +21,9 @@ export const JoinClauseRoot = styled.div`
export const JoinStrategyIcon = styled(Icon).attrs({ size: 32 })`
color: ${color("brand")};
- margin-right: ${space(1)};
+ margin-right: 6px;
+ margin-left: 2px;
+ margin-top: 6px;
`;
export const JoinTypeSelectRoot = styled.div`
@@ -62,14 +64,17 @@ export const JoinedTableControlRoot = styled.div`
export const JoinWhereConditionLabel = styled.span.attrs({ children: "where" })`
color: ${color("text-medium")};
font-weight: bold;
- margin-left: ${space(1)};
- margin-right: ${space(2)};
+ margin-top: 6px;
+ margin-left: 10px;
+ margin-right: 14px;
`;
export const JoinOnConditionLabel = styled.span.attrs({ children: "=" })`
font-weight: bold;
color: ${color("text-medium")};
- margin-right: 8px;
+ margin-left: 2px;
+ margin-right: 6px;
+ margin-top: 6px;
`;
export const RemoveJoinIcon = styled(Icon).attrs({ name: "close", size: 18 })`
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.unit.spec.js b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.unit.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..746f46a5e805abd998f6e2f23813e1022aa49f69
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep.unit.spec.js
@@ -0,0 +1,287 @@
+/* eslint-disable react/prop-types */
+import React, { useState } from "react";
+import { Provider } from "react-redux";
+import {
+ render,
+ screen,
+ fireEvent,
+ within,
+ waitForElementToBeRemoved,
+} from "@testing-library/react";
+import xhrMock from "xhr-mock";
+import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
+import { getStore } from "__support__/entities-store";
+import {
+ state,
+ ORDERS,
+ PRODUCTS,
+ SAMPLE_DATASET,
+} from "__support__/sample_dataset_fixture";
+import JoinStep from "./JoinStep";
+
+jest.setTimeout(10000);
+
+describe("Notebook Editor > Join Step", () => {
+ const TEST_QUERY = {
+ type: "query",
+ database: SAMPLE_DATASET.id,
+ query: {
+ "source-table": ORDERS.id,
+ },
+ };
+
+ function JoinStepWrapped({ initialQuery, onChange, ...props }) {
+ const [query, setQuery] = useState(initialQuery);
+ return (
+ <JoinStep
+ {...props}
+ query={query}
+ updateQuery={datasetQuery => {
+ const newQuery = query.setDatasetQuery(datasetQuery);
+ setQuery(newQuery);
+ onChange(datasetQuery);
+ }}
+ />
+ );
+ }
+
+ function setup() {
+ const query = new StructuredQuery(ORDERS.question(), TEST_QUERY);
+ const onQueryChange = jest.fn();
+
+ const TEST_STEP = {
+ id: "0:join",
+ type: "join",
+ itemIndex: 0,
+ stageIndex: 0,
+ query,
+ valid: true,
+ visible: true,
+ active: true,
+ actions: [],
+ update: jest.fn(),
+ clean: jest.fn(),
+ revert: jest.fn(),
+ };
+
+ render(
+ <Provider store={getStore({}, state)}>
+ <JoinStepWrapped
+ initialQuery={query}
+ step={TEST_STEP}
+ onChange={onQueryChange}
+ />
+ </Provider>,
+ );
+
+ return { onQueryChange };
+ }
+
+ function toFieldRef(field, joinedTable) {
+ return [
+ "field",
+ field.id,
+ joinedTable ? { "join-alias": joinedTable.display_name } : null,
+ ];
+ }
+
+ function expectedJoin({
+ fields = "all",
+ joinedTable,
+ leftField,
+ rightField,
+ }) {
+ const joinFields = Array.isArray(fields)
+ ? fields.map(field => toFieldRef(field, joinedTable))
+ : fields;
+
+ return {
+ ...TEST_QUERY,
+ query: {
+ ...TEST_QUERY.query,
+ joins: [
+ expect.objectContaining({
+ fields: joinFields,
+ "source-table": joinedTable.id,
+ condition: [
+ "=",
+ toFieldRef(leftField),
+ toFieldRef(rightField, joinedTable),
+ ],
+ }),
+ ],
+ },
+ };
+ }
+
+ async function selectTable(tableName) {
+ fireEvent.click(screen.queryByText(/Sample Dataset/i));
+ const dataSelector = await screen.findByTestId("data-selector");
+ fireEvent.click(within(dataSelector).queryByText(tableName));
+
+ await waitForElementToBeRemoved(() =>
+ screen.queryByTestId("data-selector"),
+ );
+ }
+
+ function openDimensionPicker(type) {
+ const openPickerButton = screen.getByTestId(`${type}-dimension`);
+ fireEvent.click(openPickerButton);
+ return screen.findByRole("rowgroup");
+ }
+
+ beforeEach(() => {
+ xhrMock.setup();
+ xhrMock.get("/api/database", {
+ body: JSON.stringify({
+ total: 1,
+ data: [SAMPLE_DATASET.getPlainObject()],
+ }),
+ });
+ xhrMock.get("/api/database/1/schemas", {
+ body: JSON.stringify(["PUBLIC"]),
+ });
+ xhrMock.get("/api/database/1/schema/PUBLIC", {
+ body: JSON.stringify(
+ SAMPLE_DATASET.tables.filter(table => table.schema === "PUBLIC"),
+ ),
+ });
+ });
+
+ afterEach(() => {
+ xhrMock.teardown();
+ });
+
+ it("displays a source table and suggests to pick a join table", () => {
+ setup();
+ expect(screen.queryByText("Orders")).toBeInTheDocument();
+ expect(screen.queryByText("Pick a table...")).toBeInTheDocument();
+ });
+
+ it("opens a schema browser by default", async () => {
+ setup();
+
+ fireEvent.click(screen.queryByText(/Sample Dataset/i));
+ const dataSelector = await screen.findByTestId("data-selector");
+
+ SAMPLE_DATASET.tables.forEach(table => {
+ const tableName = new RegExp(table.display_name, "i");
+ expect(within(dataSelector).queryByText(tableName)).toBeInTheDocument();
+ });
+ });
+
+ it("automatically sets join fields if possible", async () => {
+ const { onQueryChange } = setup();
+
+ await selectTable(/Products/i);
+
+ expect(screen.getByTestId("parent-dimension")).toHaveTextContent(
+ /Product ID/i,
+ );
+ expect(screen.getByTestId("join-dimension")).toHaveTextContent(/ID/i);
+ expect(onQueryChange).toHaveBeenLastCalledWith(
+ expectedJoin({
+ joinedTable: PRODUCTS,
+ leftField: ORDERS.PRODUCT_ID,
+ rightField: PRODUCTS.ID,
+ }),
+ );
+ });
+
+ it("shows a parent dimension's join field picker", async () => {
+ const ordersFields = Object.values(ORDERS.fieldsLookup());
+ setup();
+ await selectTable(/Products/i);
+
+ fireEvent.click(screen.getByTestId("parent-dimension"));
+
+ const picker = await screen.findByRole("rowgroup");
+ expect(picker).toBeInTheDocument();
+ expect(picker).toBeVisible();
+ expect(within(picker).queryByText("Order")).toBeInTheDocument();
+ ordersFields.forEach(field => {
+ expect(
+ within(picker).queryByText(field.display_name),
+ ).toBeInTheDocument();
+ });
+ });
+
+ it("can change parent dimension's join field", async () => {
+ const { onQueryChange } = setup();
+ await selectTable(/Products/i);
+ const picker = await openDimensionPicker("parent");
+
+ fireEvent.click(within(picker).getByText("Tax"));
+
+ expect(onQueryChange).toHaveBeenLastCalledWith(
+ expectedJoin({
+ joinedTable: PRODUCTS,
+ leftField: ORDERS.TAX,
+ rightField: PRODUCTS.ID,
+ }),
+ );
+ });
+
+ it("shows a join dimension's field picker", async () => {
+ const productsFields = Object.values(PRODUCTS.fieldsLookup());
+ setup();
+ await selectTable(/Products/i);
+
+ fireEvent.click(screen.getByTestId("join-dimension"));
+
+ const picker = await screen.findByRole("rowgroup");
+ expect(picker).toBeInTheDocument();
+ expect(picker).toBeVisible();
+ expect(within(picker).queryByText("Product")).toBeInTheDocument();
+ productsFields.forEach(field => {
+ expect(
+ within(picker).queryByText(field.display_name),
+ ).toBeInTheDocument();
+ });
+ });
+
+ it("can change join dimension's field", async () => {
+ const { onQueryChange } = setup();
+ await selectTable(/Products/i);
+ const picker = await openDimensionPicker("join");
+
+ fireEvent.click(within(picker).getByText("Category"));
+
+ expect(onQueryChange).toHaveBeenLastCalledWith(
+ expectedJoin({
+ joinedTable: PRODUCTS,
+ leftField: ORDERS.PRODUCT_ID,
+ rightField: PRODUCTS.CATEGORY,
+ }),
+ );
+ });
+
+ it("automatically opens dimensions picker if can't automatically set join fields", async () => {
+ setup();
+
+ await selectTable(/Reviews/i);
+
+ const picker = await screen.findByRole("rowgroup");
+ expect(picker).toBeInTheDocument();
+ expect(picker).toBeVisible();
+ expect(within(picker).queryByText("Order")).toBeInTheDocument();
+ });
+
+ it("can select fields to select from a joined table", async () => {
+ const { onQueryChange } = setup();
+ await selectTable(/Products/i);
+
+ fireEvent.click(screen.getByText("Columns"));
+ fireEvent.click(screen.getByText("Select None"));
+ fireEvent.click(screen.getByText("Category"));
+
+ expect(onQueryChange).toHaveBeenLastCalledWith(
+ expectedJoin({
+ joinedTable: PRODUCTS,
+ leftField: ORDERS.PRODUCT_ID,
+ rightField: PRODUCTS.ID,
+ fields: [PRODUCTS.CATEGORY],
+ }),
+ );
+ });
+});
diff --git a/frontend/src/metabase/reference/databases/FieldList.jsx b/frontend/src/metabase/reference/databases/FieldList.jsx
index df50df3fa58711bf27d2f5c65aa0b137498ed0f6..fd42cd47674bc735005b5db3c8d9aaf790bc91c4 100644
--- a/frontend/src/metabase/reference/databases/FieldList.jsx
+++ b/frontend/src/metabase/reference/databases/FieldList.jsx
@@ -91,6 +91,7 @@ export default class FieldList extends Component {
loadingError: PropTypes.object,
submitting: PropTypes.bool,
resetForm: PropTypes.func,
+ "data-testid": PropTypes.string,
};
render() {
@@ -123,6 +124,7 @@ export default class FieldList extends Component {
this.props,
),
)}
+ testID={this.props["data-testid"]}
>
{isEditing && (
<EditHeader
diff --git a/frontend/test/__support__/entities-store.js b/frontend/test/__support__/entities-store.js
index a452512da84edde892db2d13d182d7c70049c39b..b57d50c3887569d9c034f057b50dcaf0517cf63a 100644
--- a/frontend/test/__support__/entities-store.js
+++ b/frontend/test/__support__/entities-store.js
@@ -5,7 +5,7 @@ import promise from "redux-promise";
import { thunkWithDispatchAction } from "metabase/store";
import * as entities from "metabase/redux/entities";
-export function getStore(reducers = {}) {
+export function getStore(reducers = {}, initialState = {}) {
const reducer = combineReducers({
entities: entities.reducer,
requests: (state, action) =>
@@ -13,8 +13,6 @@ export function getStore(reducers = {}) {
...reducers,
});
- const initialState = {};
-
return createStore(
reducer,
initialState,
diff --git a/frontend/test/metabase/scenarios/question/new.cy.spec.js b/frontend/test/metabase/scenarios/question/new.cy.spec.js
index 462655059164883cdc874b7743720846a407217c..1a8b844296a63b9c4ed80f55577c24a534ef1820 100644
--- a/frontend/test/metabase/scenarios/question/new.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/new.cy.spec.js
@@ -208,7 +208,7 @@ describe("scenarios > question > new", () => {
cy.findByText("Orders, Count").click();
// Try to choose a different saved question
- cy.get("[icon=table2]").click();
+ cy.findByTestId("data-step-cell").click();
cy.findByText("Our analytics");
cy.findByText("Orders");