diff --git a/frontend/src/metabase/components/FieldValuesWidget.jsx b/frontend/src/metabase/components/FieldValuesWidget.jsx index c899a94e2ec20f338e1c8e222ea4263a8c4f025e..e9dba55ac03e188be7e0798d451e1fb774a90924 100644 --- a/frontend/src/metabase/components/FieldValuesWidget.jsx +++ b/frontend/src/metabase/components/FieldValuesWidget.jsx @@ -96,6 +96,7 @@ class FieldValuesWidgetInner extends Component { style: {}, formatOptions: {}, maxWidth: 500, + disableList: false, disableSearch: false, showOptionsInPopover: false, }; @@ -254,6 +255,7 @@ class FieldValuesWidgetInner extends Component { parameter, prefix, disableSearch, + disableList, disablePKRemappingForSearch, formatOptions, placeholder, @@ -272,11 +274,13 @@ class FieldValuesWidgetInner extends Component { }); const isLoading = loadingState === "LOADING"; - const usesListField = hasList({ - fields, - disableSearch, - options, - }); + const usesListField = + !disableList && + hasList({ + fields, + disableSearch, + options, + }); return ( <div diff --git a/frontend/src/metabase/query_builder/components/filters/FilterPopover.tsx b/frontend/src/metabase/query_builder/components/filters/FilterPopover.tsx index 19ed3aab1407bc17de57f3dc5ab2fc59da3cdaf2..ad20be0371504ba071bd610df5584e5d986e79b2 100644 --- a/frontend/src/metabase/query_builder/components/filters/FilterPopover.tsx +++ b/frontend/src/metabase/query_builder/components/filters/FilterPopover.tsx @@ -44,6 +44,7 @@ type Props = { noCommitButton?: boolean; showFieldPicker?: boolean; + showOperatorSelector?: boolean; dateShortcutOptions?: DateShortcutOptions; showCustom?: boolean; isNew?: boolean; @@ -163,6 +164,7 @@ export default class FilterPopover extends Component<Props, State> { style, query, showFieldPicker, + showOperatorSelector, fieldPickerTitle, isSidebar, isTopLevel, @@ -304,6 +306,7 @@ export default class FilterPopover extends Component<Props, State> { onFilterChange={this.handleFilterChange} onBack={onBack} showFieldPicker={showFieldPicker} + forceShowOperatorSelector={showOperatorSelector} /> <FilterPopoverPicker className={isSidebar ? "p1" : "px1 pt1 pb1"} diff --git a/frontend/src/metabase/query_builder/components/filters/FilterPopoverHeader.tsx b/frontend/src/metabase/query_builder/components/filters/FilterPopoverHeader.tsx index 0ce4d94d20509a1522f5d6c4e36d9ebd11bfc7ec..615ce18b3e3f9a9313463fb224c2a60cefe9dd07 100644 --- a/frontend/src/metabase/query_builder/components/filters/FilterPopoverHeader.tsx +++ b/frontend/src/metabase/query_builder/components/filters/FilterPopoverHeader.tsx @@ -10,6 +10,7 @@ type Props = { className?: string; showFieldPicker?: boolean; + forceShowOperatorSelector?: boolean; filter: Filter; onFilterChange: (filter: any[]) => void; onBack: () => void; @@ -19,6 +20,7 @@ type Props = { export default function FilterPopoverHeader({ className, showFieldPicker, + forceShowOperatorSelector, filter, onFilterChange, onBack, @@ -32,7 +34,7 @@ export default function FilterPopoverHeader({ const field = dimension.field(); const operator = filter.operatorName(); - const showOperatorSelector = !field.isBoolean(); + const showOperatorSelector = forceShowOperatorSelector ?? !field.isBoolean(); const showHeader = showFieldPicker || showOperatorSelector; const showOperatorSelectorOnOwnRow = isSidebar || !showFieldPicker; diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.tsx index f33bbb9b7856234f4b1986390873821de6dc1b4a..54bea2c1b83bb8334481b93432b6ce72c0c02435 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.tsx @@ -50,7 +50,7 @@ export const BulkFilterItem = ({ [filter, onAddFilter, onChangeFilter], ); - const changeOperator = (newOperator: any) => { + const changeOperator = (newOperator: string) => { handleChange((filter ?? newFilter).setOperator(newOperator)); }; @@ -94,6 +94,7 @@ export const BulkFilterItem = ({ operators={dimension.filterOperators(currentOperator)} iconName="list" tableName={tableName} + onChange={changeOperator} /> <InlineCategoryPicker query={query} @@ -150,6 +151,7 @@ export const BulkFilterItem = ({ operators={dimension.filterOperators(currentOperator)} iconName={dimension.icon() ?? undefined} tableName={tableName} + onChange={changeOperator} /> <BulkFilterSelect query={query} diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.unit.spec.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.unit.spec.tsx index 7ab4fa5ba25c9b49d63c1d8189813735e5d9c420..ea073f9b3abfd108f71f55c094a696186e3a3675 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterItem/BulkFilterItem.unit.spec.tsx @@ -2,10 +2,9 @@ // @ts-nocheck import React from "react"; -import { render, screen } from "@testing-library/react"; +import { screen } from "@testing-library/react"; import { metadata } from "__support__/sample_database_fixture"; -import { getStore } from "__support__/entities-store"; -import { Provider } from "react-redux"; +import { renderWithProviders } from "__support__/ui"; import Field from "metabase-lib/lib/metadata/Field"; import Filter from "metabase-lib/lib/queries/structured/Filter"; @@ -145,8 +144,6 @@ const pkDimension = pkField.dimension(); const fkDimension = fkField.dimension(); const textDimension = textField.dimension(); -const store = getStore(); - describe("BulkFilterItem", () => { it("renders a boolean picker for a boolean filter", () => { const testFilter = new Filter( @@ -156,7 +153,7 @@ describe("BulkFilterItem", () => { ); const changeSpy = jest.fn(); - render( + renderWithProviders( <BulkFilterItem query={query} filter={testFilter} @@ -179,17 +176,15 @@ describe("BulkFilterItem", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={testFilter} - dimension={intDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={testFilter} + dimension={intDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByTestId("value-picker"); @@ -203,17 +198,15 @@ describe("BulkFilterItem", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={testFilter} - dimension={floatDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={testFilter} + dimension={floatDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByTestId("value-picker"); }); @@ -221,17 +214,15 @@ describe("BulkFilterItem", () => { it("defaults to a between picker for float field type", () => { const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={undefined} - dimension={floatDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={undefined} + dimension={floatDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByTestId("value-picker"); screen.getByText(/between/i); @@ -247,17 +238,15 @@ describe("BulkFilterItem", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={testFilter} - dimension={categoryDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={testFilter} + dimension={categoryDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByTestId("category-picker"); }); @@ -270,17 +259,15 @@ describe("BulkFilterItem", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={testFilter} - dimension={pkDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={testFilter} + dimension={pkDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByTestId("value-picker"); }); @@ -293,17 +280,15 @@ describe("BulkFilterItem", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={testFilter} - dimension={fkDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={testFilter} + dimension={fkDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByTestId("value-picker"); }); @@ -316,17 +301,15 @@ describe("BulkFilterItem", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={testFilter} - dimension={textDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={testFilter} + dimension={textDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByTestId("value-picker"); screen.getByText("foo"); @@ -335,17 +318,15 @@ describe("BulkFilterItem", () => { it("defaults key filters to 'is' operator", () => { const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={undefined} - dimension={fkDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={undefined} + dimension={fkDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByText(/is/i); }); @@ -353,17 +334,15 @@ describe("BulkFilterItem", () => { it("defaults text filters to 'contains' operator", () => { const changeSpy = jest.fn(); - render( - <Provider store={store}> - <BulkFilterItem - query={query} - filter={undefined} - dimension={textDimension} - onAddFilter={changeSpy} - onChangeFilter={changeSpy} - onRemoveFilter={changeSpy} - /> - </Provider>, + renderWithProviders( + <BulkFilterItem + query={query} + filter={undefined} + dimension={textDimension} + onAddFilter={changeSpy} + onChangeFilter={changeSpy} + onRemoveFilter={changeSpy} + />, ); screen.getByText(/contains/i); }); diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.styled.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.styled.tsx index 40d351c45b41bed5573be76cf20c39ca40a391fa..424486db09ee83f397429e8e74d7f8f68f42a831 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.styled.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.styled.tsx @@ -1,7 +1,6 @@ import styled from "@emotion/styled"; import { color } from "metabase/lib/colors"; import { space } from "metabase/styled-components/theme"; -import Ellipsified from "metabase/core/components/Ellipsified"; export const ListRoot = styled.div` margin-bottom: 1rem; @@ -15,19 +14,10 @@ export const ListRow = styled.div` } `; -export const ListRowLabel = styled(Ellipsified)` - padding: 0.625rem 1rem 0.625rem 0; - color: ${color("black")}; - line-height: 1rem; - font-weight: bold; -`; - -export const FilterDivider = styled.div` - grid-column: 2; - border-top: 1px solid ${color("border")}; - margin: ${space(2)} 0; - &:last-of-type { - margin: 0; - display: none; +export const FilterContainer = styled.div` + &:not(:last-of-type) { + border-bottom: 1px solid ${color("border")}; + margin-bottom: ${space(2)}; + padding-bottom: ${space(2)}; } `; diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx index cd3b6cabe5da2d47f1defb994fb6c2e4a115b75a..a1d456190a6b844f4f05dabe8eab64babc0fe5e9 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterList/BulkFilterList.tsx @@ -17,12 +17,7 @@ import Filter from "metabase-lib/lib/queries/structured/Filter"; import { BulkFilterItem } from "../BulkFilterItem"; import { SegmentFilterSelect } from "../BulkFilterSelect"; import { InlineOperatorSelector } from "../InlineOperatorSelector"; -import { - ListRoot, - ListRow, - ListRowLabel, - FilterDivider, -} from "./BulkFilterList.styled"; +import { ListRoot, ListRow, FilterContainer } from "./BulkFilterList.styled"; import { sortDimensions } from "./utils"; export interface BulkFilterListProps { @@ -132,11 +127,13 @@ const BulkFilterListItem = ({ }, [filters, dimension]); return ( - <ListRow data-testid={`filter-field-${dimension.displayName()}`}> + <ListRow> {options.map((filter, index) => ( - <> + <FilterContainer + key={index} + data-testid={`filter-field-${dimension.displayName()}`} + > <BulkFilterItem - key={index} query={query} isSearch={isSearch} filter={filter} @@ -145,8 +142,7 @@ const BulkFilterListItem = ({ onChangeFilter={onChangeFilter} onRemoveFilter={onRemoveFilter} /> - <FilterDivider /> - </> + </FilterContainer> ))} </ListRow> ); diff --git a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterSelect/BulkFilterSelect.tsx b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterSelect/BulkFilterSelect.tsx index efeb3acf382d115c7a4ff146ffb682723b3cee6e..12e2a00ed01ebb65bed04069ddf4626efe03c5ad 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterSelect/BulkFilterSelect.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/BulkFilterSelect/BulkFilterSelect.tsx @@ -36,7 +36,7 @@ export const BulkFilterSelect = ({ customTrigger, handleChange, handleClear, -}: BulkFilterSelectProps): JSX.Element => { +}: BulkFilterSelectProps) => { const name = useMemo(() => { return filter?.displayName({ includeDimension: false, @@ -48,6 +48,17 @@ export const BulkFilterSelect = ({ return getNewFilter(query, dimension); }, [query, dimension]); + const hideArgumentSelector = [ + "is-null", + "not-null", + "is-empty", + "not-empty", + ].includes(filter?.operatorName()); + + if (hideArgumentSelector) { + return null; + } + return ( <TippyPopoverWithTrigger sizeToFit @@ -56,14 +67,16 @@ export const BulkFilterSelect = ({ ? customTrigger : ({ onClick, visible }) => ( <SelectFilterButton - hasValue={filter != null} + hasValue={!!filter?.isValid()} highlighted aria-label={dimension.displayName()} onClick={onClick} onClear={handleClear} isActive={visible} > - {name || t`Filter by ${dimension.displayName()}`} + {filter?.isValid() + ? name + : t`Filter by ${dimension.displayName()}`} </SelectFilterButton> ) } @@ -74,6 +87,7 @@ export const BulkFilterSelect = ({ isNew={filter == null} showCustom={false} showFieldPicker={false} + showOperatorSelector={false} dateShortcutOptions={dateShortcutOptions} onChangeFilter={handleChange} onClose={closePopover} diff --git a/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.tsx b/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.tsx index 66aceadc6b653d1da678298ad526dce0bdbbae1e..f3812c6791bf2eaa0415e06537da8a2a210d1f2a 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.tsx @@ -11,6 +11,9 @@ import { useSafeAsyncFunction } from "metabase/hooks/use-safe-async-function"; import Warnings from "metabase/query_builder/components/Warnings"; import Checkbox from "metabase/core/components/CheckBox"; +import { BulkFilterSelect } from "../BulkFilterSelect"; +import { InlineValuePicker } from "../InlineValuePicker"; + import { MAX_INLINE_CATEGORIES } from "./constants"; import { PickerContainer, @@ -18,8 +21,6 @@ import { Loading, } from "./InlineCategoryPicker.styled"; -import { BulkFilterSelect } from "../BulkFilterSelect"; - const mapStateToProps = (state: any, props: any) => { const fieldId = props.dimension?.field?.()?.id; @@ -43,6 +44,7 @@ const mapDispatchToProps = { interface InlineCategoryPickerProps { query: StructuredQuery; filter?: Filter; + tableName?: string; newFilter: Filter; dimension: Dimension; fieldValues: any[]; @@ -81,10 +83,17 @@ export function InlineCategoryPickerComponent({ }); }, [dimension, safeFetchFieldValues, shouldFetchFieldValues]); + const hasCheckboxOperator = + !filter || ["=", "!="].includes(filter?.operatorName()); + + const hasValidOptions = fieldValues.flat().find(isValidOption); + const showInlinePicker = - fieldValues.length > 0 && + hasValidOptions && fieldValues.length <= MAX_INLINE_CATEGORIES && - (!filter || filter?.operatorName() === "="); + hasCheckboxOperator; + + const showPopoverPicker = !showInlinePicker && hasCheckboxOperator; if (hasError) { return ( @@ -110,13 +119,23 @@ export function InlineCategoryPickerComponent({ ); } + if (showPopoverPicker) { + return ( + <BulkFilterSelect + query={query} + filter={filter} + dimension={dimension} + handleChange={onChange} + handleClear={onClear} + /> + ); + } + return ( - <BulkFilterSelect - query={query} - filter={filter} - dimension={dimension} + <InlineValuePicker + filter={filter ?? newFilter} + field={dimension.field()} handleChange={onChange} - handleClear={onClear} /> ); } diff --git a/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.unit.spec.tsx b/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.unit.spec.tsx index b13ab9bb41e6f4e90f690dd765d985d841fb1ec6..33601d971b5375478a6f513010771d6149487618 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/InlineCategoryPicker/InlineCategoryPicker.unit.spec.tsx @@ -4,6 +4,7 @@ import React from "react"; import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { renderWithProviders } from "__support__/ui"; import { metadata } from "__support__/sample_database_fixture"; import Field from "metabase-lib/lib/metadata/Field"; @@ -339,9 +340,9 @@ describe("InlineCategoryPicker", () => { expect(fetchSpy).not.toHaveBeenCalled(); }); - it("should fall back to bulk filter if the filter operator is not =", async () => { + it("should fall back to a bulk (popover) picker if there are many options", () => { const testFilter = new Filter( - ["!=", ["field", smallCategoryField.id, null], undefined], + ["=", ["field", largeCategoryField.id, null], undefined], null, query, ); @@ -354,9 +355,9 @@ describe("InlineCategoryPicker", () => { filter={testFilter} newFilter={testFilter} onChange={changeSpy} - fieldValues={smallCategoryField.values} + fieldValues={largeCategoryField.values} fetchFieldValues={fetchSpy} - dimension={smallDimension} + dimension={largeDimension} onClear={changeSpy} />, ); @@ -364,4 +365,37 @@ describe("InlineCategoryPicker", () => { expect(screen.queryByTestId("category-picker")).not.toBeInTheDocument(); expect(screen.queryByTestId("select-button")).toBeInTheDocument(); }); + + const fieldSizes = [ + { name: "large", field: largeCategoryField, dimension: largeDimension }, + { name: "small", field: smallCategoryField, dimension: smallDimension }, + ]; + + fieldSizes.forEach(({ name, field, dimension }) => { + it(`should fall back to value picker if the filter operator is not = or != with a ${name} set of field values`, () => { + const testFilter = new Filter( + ["contains", ["field", field.id, null], undefined], + null, + query, + ); + const changeSpy = jest.fn(); + const fetchSpy = jest.fn(); + + renderWithProviders( + <InlineCategoryPickerComponent + query={query} + filter={testFilter} + newFilter={testFilter} + onChange={changeSpy} + fieldValues={field.values} + fetchFieldValues={fetchSpy} + dimension={dimension} + onClear={changeSpy} + />, + ); + + expect(screen.queryByTestId("category-picker")).not.toBeInTheDocument(); + expect(screen.queryByTestId("value-picker")).toBeInTheDocument(); + }); + }); }); diff --git a/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.styled.ts b/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.styled.ts index c49059392e3bd9f5a6a418b214627d9a3f97040e..21ac51c72330e3fe4a6d9acaa63f7763c4fe3adf 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.styled.ts +++ b/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.styled.ts @@ -2,14 +2,9 @@ import styled from "@emotion/styled"; import { space } from "metabase/styled-components/theme"; import { color } from "metabase/lib/colors"; -import OperatorSelectorComponent from "metabase/query_builder/components/filters/OperatorSelector"; import FieldValuesWidget from "metabase/components/FieldValuesWidget"; import NumericInput from "metabase/core/components/NumericInput"; -export const OperatorSelector = styled(OperatorSelectorComponent)` - margin-bottom: ${space(1)}; -`; - export const ArgumentSelector = styled(FieldValuesWidget)` min-height: 55px; `; diff --git a/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.tsx b/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.tsx index 49c52bb5377f480544fede8b93f803d1dde2c4da..36fb0ca7c0f5373441b344ab1d32aa999c514b80 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.tsx @@ -1,10 +1,10 @@ import React, { useCallback, useMemo } from "react"; +import { t } from "ttag"; + import Filter from "metabase-lib/lib/queries/structured/Filter"; import Field from "metabase-lib/lib/metadata/Field"; -import { t } from "ttag"; import { - OperatorSelector, ArgumentSelector, ValuesPickerContainer, BetweenContainer, @@ -83,6 +83,7 @@ function ValuesInput({ fields={[field]} multi={!!filter?.operator()?.multi} showOptionsInPopover + disableList /> ); } diff --git a/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.unit.spec.tsx b/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.unit.spec.tsx index 51869fadbdc2b0e799e82482fd50cb186c0fbacd..bb03f1aabe92c9b2ec2fb39639d981b599153172 100644 --- a/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/filters/modals/InlineValuePicker/InlineValuePicker.unit.spec.tsx @@ -1,11 +1,10 @@ // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-nocheck import React from "react"; -import { render, screen, waitFor } from "@testing-library/react"; +import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { Provider } from "react-redux"; +import { renderWithProviders } from "__support__/ui"; -import { getStore } from "__support__/entities-store"; import { metadata } from "__support__/sample_database_fixture"; import Field from "metabase-lib/lib/metadata/Field"; @@ -77,7 +76,6 @@ const card = { const question = new Question(card, metadata); const query = question.query(); -const store = getStore(); describe("InlineValuePicker", () => { it("renders an inline value picker with values fields", () => { @@ -88,14 +86,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={pkField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={pkField} + handleChange={changeSpy} + />, ); screen.getByTestId("value-picker"); @@ -110,14 +106,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={pkField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={pkField} + handleChange={changeSpy} + />, ); screen.getByText("777"); screen.getByText("888"); @@ -131,14 +125,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={textField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={textField} + handleChange={changeSpy} + />, ); screen.getByText("fooBarBaz"); @@ -153,14 +145,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={pkField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={pkField} + handleChange={changeSpy} + />, ); const textInput = screen.getByPlaceholderText("Enter an ID"); @@ -181,14 +171,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={pkField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={pkField} + handleChange={changeSpy} + />, ); // click remove on the first data item, which is 777 @@ -206,14 +194,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(newFilter => (testFilter = newFilter)); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={textField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={textField} + handleChange={changeSpy} + />, ); const textInput = screen.getByPlaceholderText("Enter some text"); @@ -232,14 +218,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={textField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={textField} + handleChange={changeSpy} + />, ); const textInput = screen.getByPlaceholderText("Enter some text"); @@ -258,14 +242,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={pkField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={pkField} + handleChange={changeSpy} + />, ); screen.getByPlaceholderText("Min"); @@ -283,14 +265,12 @@ describe("InlineValuePicker", () => { ); const changeSpy = jest.fn(); - render( - <Provider store={store}> - <InlineValuePicker - filter={testFilter} - field={textField} - handleChange={changeSpy} - /> - </Provider>, + renderWithProviders( + <InlineValuePicker + filter={testFilter} + field={textField} + handleChange={changeSpy} + />, ); expect( diff --git a/frontend/test/__support__/e2e/helpers/e2e-bi-basics-helpers.js b/frontend/test/__support__/e2e/helpers/e2e-bi-basics-helpers.js index 5f9b42743473f943d52cec6e8e665c264c7ef314..e157d2f6ca45de2d5d9b73278b0e20b339493b77 100644 --- a/frontend/test/__support__/e2e/helpers/e2e-bi-basics-helpers.js +++ b/frontend/test/__support__/e2e/helpers/e2e-bi-basics-helpers.js @@ -8,20 +8,26 @@ export function filter({ mode } = {}) { initiateAction("Filter", mode); } -export function filterField(fieldName, { operator, value, placeholder } = {}) { +export function filterField( + fieldName, + { operator, value, placeholder, order } = {}, +) { if (operator) { - changeOperator(getFilterField(fieldName), operator); + changeOperator(getFilterField(fieldName, order), operator); } if (value) { - changeValue(getFilterField(fieldName), value, placeholder); + changeValue(getFilterField(fieldName, order), value, placeholder); } - return getFilterField(fieldName); + return getFilterField(fieldName, order); } -export function filterFieldPopover(fieldName, { value, placeholder } = {}) { - getFilterField(fieldName).within(() => { +export function filterFieldPopover( + fieldName, + { value, placeholder, order } = {}, +) { + getFilterField(fieldName, order).within(() => { cy.findByTestId("select-button").click(); }); @@ -31,8 +37,8 @@ export function filterFieldPopover(fieldName, { value, placeholder } = {}) { return popover(); } -function getFilterField(fieldName) { - return cy.findByTestId(`filter-field-${fieldName}`); +function getFilterField(fieldName, order = 0) { + return cy.findAllByTestId(`filter-field-${fieldName}`).eq(order); } function changeOperator(subject, operator) { diff --git a/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js b/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js index cd1e61244efeb0a0ef8e51e2c89cdfd51e29c81f..3e0a0bf15b62002a8a40483099ef02645ed9a03e 100644 --- a/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js +++ b/frontend/test/metabase/scenarios/filters/filter-bulk.cy.spec.js @@ -5,10 +5,10 @@ import { setupBooleanQuery, filter, filterField, + filterFieldPopover, } from "__support__/e2e/helpers"; import { SAMPLE_DB_ID } from "__support__/e2e/cypress_data"; import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database"; -import { filterFieldPopover } from "../../../__support__/e2e/helpers/e2e-bi-basics-helpers"; const { ORDERS_ID, ORDERS, PEOPLE_ID, PRODUCTS_ID } = SAMPLE_DATABASE; @@ -147,15 +147,7 @@ describe("scenarios > filters > bulk filtering", () => { visitQuestionAdhoc(filteredQuestionDetails); filter(); - modal().within(() => { - cy.findByText("30").click(); - }); - - popover().within(() => { - cy.findByRole("textbox").click().type("30").clear().type("25"); - - cy.button("Update filter").click(); - }); + filterField("Quantity", { order: 1, value: "{backspace}{backspace}25" }); applyFilters(); @@ -168,11 +160,8 @@ describe("scenarios > filters > bulk filtering", () => { visitQuestionAdhoc(filteredQuestionDetails); filter(); - modal().within(() => { - cy.findByText("30") - .parent() - .within(() => cy.icon("close").click()); - }); + filterField("Quantity", { order: 1, value: "{backspace}{backspace}" }); + applyFilters(); cy.findByText("Quantity is greater than 20").should("be.visible");