diff --git a/frontend/src/metabase/query_builder/components/filters/FilterPopover/FilterPopoverPicker.tsx b/frontend/src/metabase/query_builder/components/filters/FilterPopover/FilterPopoverPicker.tsx index 4b20c37a0e33c312fc4979f35e567ce5dbe5ce45..262bbe15472d8ed18a2443f18d5f7d51561f3c5c 100644 --- a/frontend/src/metabase/query_builder/components/filters/FilterPopover/FilterPopoverPicker.tsx +++ b/frontend/src/metabase/query_builder/components/filters/FilterPopover/FilterPopoverPicker.tsx @@ -3,7 +3,7 @@ import { Component } from "react"; import Filter from "metabase-lib/queries/structured/Filter"; import TimePicker from "../pickers/TimePicker"; import BooleanPicker from "../pickers/BooleanPicker"; -import DefaultPicker from "../pickers/DefaultPicker"; +import { DefaultPicker } from "../pickers/DefaultPicker"; type Props = { className?: string; diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.styled.tsx b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.styled.tsx similarity index 100% rename from frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.styled.tsx rename to frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.styled.tsx diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.tsx similarity index 79% rename from frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx rename to frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.tsx index a965432bc829f35ae9332bc2d2d59c841c2e448f..3e144ed85c9a00aa1d6b7e41c7e1f71cc255f50a 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.tsx @@ -1,12 +1,13 @@ -/* eslint-disable react/prop-types */ -/* eslint-disable react/jsx-key */ import PropTypes from "prop-types"; import cx from "classnames"; import { t } from "ttag"; +import type { ReactElement } from "react"; import FieldValuesWidget from "metabase/components/FieldValuesWidget"; +import type { DatasetColumn, FieldId, RowValue } from "metabase-types/api"; import { getCurrencySymbol } from "metabase/lib/formatting"; +import type Filter from "metabase-lib/queries/structured/Filter"; import { getFilterArgumentFormatOptions, @@ -14,9 +15,9 @@ import { } from "metabase-lib/operators/utils"; import { isCurrency } from "metabase-lib/types/utils/isa"; import { getColumnKey } from "metabase-lib/queries/utils/get-column-key"; -import TextPicker from "./TextPicker"; -import SelectPicker from "./SelectPicker"; -import NumberPicker from "./NumberPicker"; +import TextPicker from "../TextPicker"; +import SelectPicker from "../SelectPicker"; +import NumberPicker from "../NumberPicker"; import { BetweenLayoutContainer, @@ -41,7 +42,17 @@ const defaultLayoutPropTypes = { fieldWidgets: PropTypes.array, }; -export default function DefaultPicker({ +export interface DefaultPickerProps { + filter: Filter; + setValue: (index: number, value: RowValue) => void; + setValues: (values: RowValue[]) => void; + onCommit: (filter: Filter) => void; + className?: string; + minWidth?: number | null; + maxWidth?: number | null; + checkedColor?: string; +} +export function DefaultPicker({ filter, setValue, setValues, @@ -50,7 +61,7 @@ export default function DefaultPicker({ minWidth, maxWidth, checkedColor, -}) { +}: DefaultPickerProps) { const operator = filter.operator(); if (!operator) { return <div className={className} />; @@ -66,10 +77,13 @@ export default function DefaultPicker({ const visualizationSettings = filter?.query()?.question()?.settings(); - const key = getColumnKey(dimension.column()); + const key = dimension?.column?.() + ? getColumnKey(dimension.column() as DatasetColumn) + : ""; + const columnSettings = visualizationSettings?.column_settings?.[key]; - const fieldMetadata = field?.metadata?.fields[field?.id]; + const fieldMetadata = field?.metadata?.fields[field?.id as FieldId]; const fieldSettings = { ...(fieldMetadata?.settings ?? {}), ...(columnSettings ?? {}), @@ -88,15 +102,16 @@ export default function DefaultPicker({ let values, onValuesChange; if (operator.multi) { values = filter.arguments(); - onValuesChange = values => setValues(values); + onValuesChange = (values: RowValue[]) => setValues(values); } else { values = [filter.arguments()[index]]; - onValuesChange = values => setValue(index, values[0]); + onValuesChange = (values: RowValue[]) => setValue(index, values[0]); } if (operatorField.type === "hidden") { return null; } else if (operatorField.type === "select") { + // unclear, but this may be a dead code path return ( <SelectPicker key={index} @@ -112,11 +127,12 @@ export default function DefaultPicker({ // get the underling field if the query is nested let underlyingField = field; let sourceField; - while ((sourceField = underlyingField.sourceField())) { + while ((sourceField = underlyingField?.sourceField())) { underlyingField = sourceField; } return ( <FieldValuesWidget + key={index} className="input" value={values} onChange={onValuesChange} @@ -175,7 +191,6 @@ export default function DefaultPicker({ return ( <DefaultPickerContainer data-testid="default-picker-container" - limitHeight className={cx(className, "PopoverBody--marginBottom")} > {layout} @@ -185,13 +200,14 @@ export default function DefaultPicker({ DefaultPicker.propTypes = defaultPickerPropTypes; -const DefaultLayout = ({ className, fieldWidgets }) => ( - <div className={className}> +const DefaultLayout = ({ + fieldWidgets, +}: { + fieldWidgets: (ReactElement | null)[]; +}) => ( + <div> {fieldWidgets.map((fieldWidget, index) => ( - <div - key={index} - className={index < fieldWidgets.length - 1 ? "mb1" : null} - > + <div key={index} className={index < fieldWidgets.length - 1 ? "mb1" : ""}> {fieldWidget} </div> ))} @@ -200,7 +216,11 @@ const DefaultLayout = ({ className, fieldWidgets }) => ( DefaultLayout.propTypes = defaultLayoutPropTypes; -const BetweenLayout = ({ className, fieldWidgets }) => { +const BetweenLayout = ({ + fieldWidgets, +}: { + fieldWidgets: (ReactElement | null)[]; +}) => { const [left, right] = fieldWidgets; return ( diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.unit.spec.tsx b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..0ba1f1ec0c00f6dd3a816ed6a723fa66f3c5126e --- /dev/null +++ b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/DefaultPicker.unit.spec.tsx @@ -0,0 +1,178 @@ +import userEvent from "@testing-library/user-event"; +import { renderWithProviders, screen, waitFor } from "__support__/ui"; +import { createMockMetadata } from "__support__/metadata"; +import { setupFieldValuesEndpoints } from "__support__/server-mocks"; + +import { checkNotNull } from "metabase/core/utils/types"; + +import { + createSampleDatabase, + PRODUCTS_ID, + SAMPLE_DB_ID, + PRODUCTS, +} from "metabase-types/api/mocks/presets"; + +import StructuredQuery from "metabase-lib/queries/StructuredQuery"; + +import { DefaultPicker, DefaultPickerProps } from "./DefaultPicker"; + +const metadata = createMockMetadata({ + databases: [createSampleDatabase()], +}); + +const ordersTable = checkNotNull(metadata.table(PRODUCTS_ID)); + +const makeQuery = (query = {}): StructuredQuery => { + return ordersTable + .question() + .setDatasetQuery({ + type: "query", + query: { + "source-table": PRODUCTS_ID, + ...query, + }, + database: SAMPLE_DB_ID, + }) + .query() as StructuredQuery; +}; + +const numericQuery = makeQuery({ + filter: ["=", ["field", PRODUCTS.ID, null], 42], +}); +const stringQuery = makeQuery({ + filter: ["=", ["field", PRODUCTS.TITLE, null], "Ugly Shoes"], +}); + +async function setup(options: Partial<DefaultPickerProps> = {}) { + const setValueSpy = jest.fn(); + const setValuesSpy = jest.fn(); + const onCommitSpy = jest.fn(); + + setupFieldValuesEndpoints({ + field_id: PRODUCTS.ID, + values: [[42], [43], [44], [56]], + has_more_values: false, + }); + + setupFieldValuesEndpoints({ + field_id: PRODUCTS.TITLE, + values: [["Fancy Shoes"], ["Ugly Shoes"], ["Fancy Boots"], ["Ugly Boots"]], + has_more_values: false, + }); + + renderWithProviders( + <DefaultPicker + filter={stringQuery.filters()[0]} + setValue={setValueSpy} + setValues={setValuesSpy} + onCommit={onCommitSpy} + {...options} + />, + ); + + await screen.findByTestId("default-picker-container"); + await waitFor(() => + expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument(), + ); + + return { setValueSpy, setValuesSpy, onCommitSpy }; +} + +describe("Filters > DefaultPicker", () => { + it("should render the default picker", () => { + setup(); + + expect(screen.getByTestId("default-picker-container")).toBeInTheDocument(); + }); + + it("should render a field values widget for a numeric filter", async () => { + await setup({ filter: numericQuery.filters()[0] }); + + expect( + await screen.findByTestId("field-values-widget"), + ).toBeInTheDocument(); + expect(screen.getByRole("textbox")).toBeInTheDocument(); + expect(screen.getByText("42")).toBeInTheDocument(); + }); + + it("should render a field values widget for a string filter", async () => { + await setup({ filter: stringQuery.filters()[0] }); + + expect( + await screen.findByTestId("field-values-widget"), + ).toBeInTheDocument(); + + expect(screen.getByRole("textbox")).toBeInTheDocument(); + expect(screen.getByText("Ugly Shoes")).toBeInTheDocument(); + }); + + it("lists possible field values for a string filter", async () => { + await setup({ filter: stringQuery.filters()[0] }); + + expect( + await screen.findByTestId("field-values-widget"), + ).toBeInTheDocument(); + expect(screen.getByRole("textbox")).toBeInTheDocument(); + const productTitles = [ + "Fancy Shoes", + "Ugly Shoes", + "Fancy Boots", + "Ugly Boots", + ]; + + productTitles.forEach(productTitle => { + expect(screen.getByText(productTitle)).toBeInTheDocument(); + }); + }); + + it("should render numeric pickers for a between filter", async () => { + const query = makeQuery({ + filter: ["between", ["field", PRODUCTS.ID, null], 42, 49], + }); + await setup({ filter: query.filters()[0] }); + + expect(screen.getAllByTestId("number-picker")).toHaveLength(2); + expect(screen.getAllByRole("textbox")).toHaveLength(2); + expect(screen.getByDisplayValue("42")).toBeInTheDocument(); + expect(screen.getByDisplayValue("49")).toBeInTheDocument(); + }); + + it("should update values for a multi-value filter", async () => { + const { setValuesSpy } = await setup({ filter: stringQuery.filters()[0] }); + + const input = screen.getByRole("textbox"); + + userEvent.type(input, "Fancy Sandals"); + + expect(setValuesSpy).toHaveBeenLastCalledWith([ + "Ugly Shoes", + "Fancy Sandals", + ]); + }); + + it("should update value for a single value filter", async () => { + const query = makeQuery({ filter: [">", ["field", PRODUCTS.ID, null], 1] }); + const { setValueSpy } = await setup({ filter: query.filters()[0] }); + + const input = screen.getByRole("textbox"); + + userEvent.type(input, "25"); + // index, value + expect(setValueSpy).toHaveBeenLastCalledWith(0, 125); + }); + + it("should call onCommit when enter is pressed for between filters", async () => { + const query = makeQuery({ + filter: ["between", ["field", PRODUCTS.ID, null], 42, 49], + }); + const { onCommitSpy } = await setup({ filter: query.filters()[0] }); + + expect(screen.getAllByTestId("number-picker")).toHaveLength(2); + + const input = screen.getAllByRole("textbox")[0]; + + userEvent.type(input, "1{enter}"); + + expect(onCommitSpy).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/index.ts b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..6b40368b4bee350b51abad33d04834c7ecc1a57a --- /dev/null +++ b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker/index.ts @@ -0,0 +1 @@ +export * from "./DefaultPicker"; diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx index bf4c25496050f5ec7aea17392068b0710461682f..2b2f8fbd3fd217a9954bacf6803835ff839c9825 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/NumberPicker.jsx @@ -48,6 +48,7 @@ export default class NumberPicker extends Component { return ( <TextPicker {...this.props} + data-testid="number-picker" isSingleLine prefix={this.props.prefix} values={values} diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx index 3a35a6325d3ce17263dc368938aae6bb83ee8729..a9930fe373abfa91921f4e16f395903ebfafadf2 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/SelectPicker.jsx @@ -87,7 +87,7 @@ export default class SelectPicker extends Component { } return ( - <div> + <div data-testid="select-picker"> {validOptions.length <= 10 && !regex ? null : ( <div className="px1 pt1"> <ListSearchField diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx index 0234f4375ec1ddb5659526ac4ba2254c9d48768e..1506fbc42e784cb5ea5fbf2667b0f4b168c27549 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/TextPicker.jsx @@ -49,8 +49,15 @@ export default class TextPicker extends Component { } render() { - const { validations, multi, onCommit, isSingleLine, autoFocus, prefix } = - this.props; + const { + validations, + multi, + onCommit, + isSingleLine, + autoFocus, + prefix, + "data-testid": testId, + } = this.props; const hasInvalidValues = _.some(validations, v => v === false); const commitOnEnter = e => { @@ -60,7 +67,7 @@ export default class TextPicker extends Component { }; return ( - <div> + <div data-testid={testId ?? "text-picker"}> <div className="FilterInput px1 pt1 relative flex align-center"> {!!prefix && ( <span