From dab738c6cf55d00326269e9807962bc139e21da7 Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Tue, 4 Feb 2020 16:27:50 -0800
Subject: [PATCH] Clean up dashboard filters with multiple distinct fields
(#11741) (#11858)
---
.../src/metabase-lib/lib/metadata/Field.js | 24 -
.../components/database/MetadataHeader.jsx | 2 +-
.../containers/MetadataEditorApp.jsx | 7 +-
.../metabase/components/FieldValuesWidget.jsx | 199 ++++---
.../components/ParameterValueWidget.jsx | 23 +-
.../components/widgets/CategoryWidget.jsx | 113 ----
.../widgets/ParameterFieldWidget.jsx | 23 +-
.../filters/pickers/DefaultPicker.jsx | 4 +-
.../components/FieldValuesWidget.unit.spec.js | 88 ++-
.../widgets/CategoryWidget.e2e.spec.js | 84 ---
.../parameters/parameters.e2e.spec.js | 511 ------------------
11 files changed, 220 insertions(+), 858 deletions(-)
delete mode 100644 frontend/src/metabase/parameters/components/widgets/CategoryWidget.jsx
delete mode 100644 frontend/test/metabase/parameters/components/widgets/CategoryWidget.e2e.spec.js
delete mode 100644 frontend/test/metabase/parameters/parameters.e2e.spec.js
diff --git a/frontend/src/metabase-lib/lib/metadata/Field.js b/frontend/src/metabase-lib/lib/metadata/Field.js
index 082b3cfb2f5..f981ddb88fb 100644
--- a/frontend/src/metabase-lib/lib/metadata/Field.js
+++ b/frontend/src/metabase-lib/lib/metadata/Field.js
@@ -287,30 +287,6 @@ export default class Field extends Base {
return this.isString();
}
- /**
- * Returns the field to be searched for this field, either the remapped field or itself
- */
- parameterSearchField(): ?Field {
- const remappedField = this.remappedField();
- if (remappedField && remappedField.isSearchable()) {
- return remappedField;
- }
- if (this.isSearchable()) {
- return this;
- }
- return null;
- }
-
- filterSearchField(): ?Field {
- if (this.isPK()) {
- if (this.isSearchable()) {
- return this;
- }
- } else {
- return this.parameterSearchField();
- }
- }
-
column(extra = {}) {
return this.dimension().column({ source: "fields", ...extra });
}
diff --git a/frontend/src/metabase/admin/datamodel/components/database/MetadataHeader.jsx b/frontend/src/metabase/admin/datamodel/components/database/MetadataHeader.jsx
index e796a1c9340..d0a94418c12 100644
--- a/frontend/src/metabase/admin/datamodel/components/database/MetadataHeader.jsx
+++ b/frontend/src/metabase/admin/datamodel/components/database/MetadataHeader.jsx
@@ -23,7 +23,7 @@ export default class MetadataHeader extends Component {
setDatabaseIdIfUnset() {
const { databaseId, databases = [], selectDatabase } = this.props;
if (databaseId === undefined && databases.length > 0) {
- selectDatabase(databases[0]);
+ selectDatabase(databases[0], true);
}
}
diff --git a/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx b/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx
index 4cf2b35b856..0ac27673cd0 100644
--- a/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx
+++ b/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx
@@ -1,7 +1,7 @@
import React, { Component } from "react";
import PropTypes from "prop-types";
import { connect } from "react-redux";
-import { push } from "react-router-redux";
+import { push, replace } from "react-router-redux";
import { t } from "ttag";
import MetabaseAnalytics from "metabase/lib/analytics";
@@ -31,7 +31,10 @@ const mapStateToProps = (state, { params }) => {
};
const mapDispatchToProps = {
- selectDatabase: ({ id }) => push("/admin/datamodel/database/" + id),
+ selectDatabase: ({ id }, shouldReplace) =>
+ shouldReplace
+ ? replace(`/admin/datamodel/database/${id}`)
+ : push(`/admin/datamodel/database/${id}`),
selectTable: ({ id, db_id }) =>
push(`/admin/datamodel/database/${db_id}/table/${id}`),
updateField: field => Fields.actions.update(field),
diff --git a/frontend/src/metabase/components/FieldValuesWidget.jsx b/frontend/src/metabase/components/FieldValuesWidget.jsx
index e81070c740e..9c438d04a2d 100644
--- a/frontend/src/metabase/components/FieldValuesWidget.jsx
+++ b/frontend/src/metabase/components/FieldValuesWidget.jsx
@@ -5,7 +5,7 @@ import { connect } from "react-redux";
import { t, jt } from "ttag";
import TokenField from "metabase/components/TokenField";
-import RemappedValue from "metabase/containers/RemappedValue";
+import ValueComponent from "metabase/components/Value";
import LoadingSpinner from "metabase/components/LoadingSpinner";
import AutoExpanding from "metabase/hoc/AutoExpanding";
@@ -13,7 +13,7 @@ import AutoExpanding from "metabase/hoc/AutoExpanding";
import { MetabaseApi } from "metabase/services";
import { addRemappings, fetchFieldValues } from "metabase/redux/metadata";
import { defer } from "metabase/lib/promise";
-import { debounce } from "underscore";
+import { debounce, zip } from "underscore";
import { stripId } from "metabase/lib/formatting";
import Fields from "metabase/entities/fields";
@@ -31,18 +31,21 @@ const mapDispatchToProps = {
fetchFieldValues,
};
-function mapStateToProps(state, { field }) {
- const selectedField =
- field && Fields.selectors.getObject(state, { entityId: field.id });
- // try and use the selected field, but fall back to the one passed
- return { field: selectedField || field };
+function mapStateToProps(state, { fields = [] }) {
+ // try and use the selected fields, but fall back to the ones passed
+ return {
+ fields: fields.map(
+ field =>
+ Fields.selectors.getObject(state, { entityId: field.id }) || field,
+ ),
+ };
}
type Props = {
value: Value[],
onChange: (value: Value[]) => void,
- field: Field,
- searchField?: Field,
+ fields: Field[],
+ disablePKRemappingForSearch?: boolean,
multi?: boolean,
autoFocus?: boolean,
color?: string,
@@ -91,9 +94,9 @@ export class FieldValuesWidget extends Component {
};
componentWillMount() {
- const { field, fetchFieldValues } = this.props;
- if (field.has_field_values === "list") {
- fetchFieldValues(field.id);
+ const { fields, fetchFieldValues } = this.props;
+ if (fields.every(field => field.has_field_values === "list")) {
+ fields.forEach(field => fetchFieldValues(field.id));
}
}
@@ -104,13 +107,24 @@ export class FieldValuesWidget extends Component {
}
hasList() {
- const { field } = this.props;
- return field.has_field_values === "list" && field.values;
+ return this.props.fields.every(
+ field => field.has_field_values === "list" && field.values,
+ );
}
isSearchable() {
- const { field, searchField } = this.props;
- return searchField && field.has_field_values === "search";
+ const { fields } = this.props;
+ return (
+ // search is available if:
+ // all fields have a valid search field
+ fields.every(this.searchField) &&
+ // at least one field is set to display as "search"
+ fields.some(f => f.has_field_values === "search") &&
+ // and all fields are either "search" or "list"
+ fields.every(
+ f => f.has_field_values === "search" || f.has_field_values === "list",
+ )
+ );
}
onInputChange = (value: string) => {
@@ -121,30 +135,48 @@ export class FieldValuesWidget extends Component {
return value;
};
- search = async (value: string, cancelled: Promise<void>) => {
- const { field, searchField, maxResults } = this.props;
+ searchField = (field: Field) => {
+ if (this.props.disablePKRemappingForSearch && field.isPK()) {
+ return field.isSearchable() ? field : null;
+ }
+
+ const remappedField = field.remappedField();
+ if (remappedField && remappedField.isSearchable()) {
+ return remappedField;
+ }
+ return field.isSearchable() ? field : null;
+ };
- if (!field || !searchField || !value) {
+ search = async (value: string, cancelled: Promise<void>) => {
+ if (!value) {
return;
}
- const fieldId = (field.target || field).id;
- const searchFieldId = searchField.id;
- const results = await MetabaseApi.field_search(
- {
- value,
- fieldId,
- searchFieldId,
- limit: maxResults,
- },
- { cancelled },
+ const { fields } = this.props;
+
+ const allResults = await Promise.all(
+ fields.map(field =>
+ MetabaseApi.field_search(
+ {
+ value,
+ fieldId: field.id,
+ // $FlowFixMe all fields have a search field if we're searching
+ searchFieldId: this.searchField(field).id,
+ limit: this.props.maxResults,
+ },
+ { cancelled },
+ ),
+ ),
);
- if (results && field.remappedField() === searchField) {
- // $FlowFixMe: addRemappings provided by @connect
- this.props.addRemappings(field.id, results);
+ for (const [field, result] of zip(fields, allResults)) {
+ if (result && field.remappedField() === this.searchField(field)) {
+ // $FlowFixMe: addRemappings provided by @connect
+ this.props.addRemappings(field.id, result);
+ }
}
- return results;
+
+ return dedupeValues(allResults);
};
_search = (value: string) => {
@@ -208,7 +240,7 @@ export class FieldValuesWidget extends Component {
isFocused,
isAllSelected,
}: LayoutRendererProps) {
- const { alwaysShowOptions, field, searchField } = this.props;
+ const { alwaysShowOptions, fields } = this.props;
const { loadingState } = this.state;
if (alwaysShowOptions || isFocused) {
if (optionsList) {
@@ -221,38 +253,61 @@ export class FieldValuesWidget extends Component {
if (loadingState === "LOADING") {
return <LoadingState />;
} else if (loadingState === "LOADED") {
- return <NoMatchState field={searchField || field} />;
+ // $FlowFixMe all fields have a search field if this.isSearchable()
+ return <NoMatchState fields={fields.map(this.searchField)} />;
}
}
}
}
+ renderValue = (value: Value, options: FormattingOptions) => {
+ const { fields, formatOptions } = this.props;
+ return (
+ <ValueComponent
+ value={value}
+ column={fields[0]}
+ maximumFractionDigits={20}
+ remap={fields.length === 1}
+ {...formatOptions}
+ // $FlowFixMe
+ {...options}
+ />
+ );
+ };
+
render() {
const {
value,
onChange,
- field,
- searchField,
+ fields,
multi,
autoFocus,
color,
className,
style,
- formatOptions,
optionsMaxHeight,
} = this.props;
const { loadingState } = this.state;
let { placeholder } = this.props;
if (!placeholder) {
+ const [field] = fields;
if (this.hasList()) {
placeholder = t`Search the list`;
- } else if (this.isSearchable() && searchField) {
- const searchFieldName =
- stripId(searchField.display_name) || searchField.display_name;
- placeholder = t`Search by ${searchFieldName}`;
- if (field.isID() && field !== searchField) {
- placeholder += t` or enter an ID`;
+ } else if (this.isSearchable()) {
+ const names = new Set(
+ // $FlowFixMe all fields have a search field if this.isSearchable()
+ fields.map(field => stripId(this.searchField(field).display_name)),
+ );
+ if (names.size > 1) {
+ placeholder = t`Search`;
+ } else {
+ // $FlowFixMe
+ const [name] = names;
+ placeholder = t`Search by ${name}`;
+ if (field.isID() && field !== this.searchField(field)) {
+ placeholder += t` or enter an ID`;
+ }
}
} else {
if (field.isID()) {
@@ -267,7 +322,7 @@ export class FieldValuesWidget extends Component {
let options = [];
if (this.hasList()) {
- options = field.values;
+ options = dedupeValues(fields.map(field => field.values));
} else if (this.isSearchable() && loadingState === "LOADED") {
options = this.state.options;
} else {
@@ -302,25 +357,12 @@ export class FieldValuesWidget extends Component {
options={options}
// $FlowFixMe
valueKey={0}
- valueRenderer={value => (
- <RemappedValue
- value={value}
- column={field}
- {...formatOptions}
- maximumFractionDigits={20}
- compact={false}
- autoLoad={true}
- />
- )}
- optionRenderer={option => (
- <RemappedValue
- value={option[0]}
- column={field}
- maximumFractionDigits={20}
- autoLoad={false}
- {...formatOptions}
- />
- )}
+ valueRenderer={value =>
+ this.renderValue(value, { autoLoad: true, compact: false })
+ }
+ optionRenderer={option =>
+ this.renderValue(option[0], { autoLoad: false })
+ }
layoutRenderer={props => (
<div>
{props.valuesList}
@@ -346,7 +388,7 @@ export class FieldValuesWidget extends Component {
return null;
}
// if the field is numeric we need to parse the string into an integer
- if (field.isNumeric()) {
+ if (fields[0].isNumeric()) {
if (/^-?\d+(\.\d+)?$/.test(v)) {
return parseFloat(v);
} else {
@@ -361,6 +403,12 @@ export class FieldValuesWidget extends Component {
}
}
+function dedupeValues(valuesList) {
+ // $FlowFixMe
+ const uniqueValueMap = new Map(valuesList.flat().map(o => [o[0], o]));
+ return Array.from(uniqueValueMap.values());
+}
+
const LoadingState = () => (
<div
className="flex layout-centered align-center border-bottom"
@@ -370,13 +418,20 @@ const LoadingState = () => (
</div>
);
-const NoMatchState = ({ field }) => (
- <OptionsMessage
- message={jt`No matching ${(
- <strong> {field.display_name} </strong>
- )} found.`}
- />
-);
+const NoMatchState = ({ fields }: { fields: Field[] }) => {
+ if (fields.length > 1) {
+ // if there is more than one field, don't name them
+ return <OptionsMessage message={t`No matching result`} />;
+ }
+ const [{ display_name }] = fields;
+ return (
+ <OptionsMessage
+ message={jt`No matching ${(
+ <strong> {display_name} </strong>
+ )} found.`}
+ />
+ );
+};
const EveryOptionState = () => (
<OptionsMessage
diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
index b50cf23b87a..94b854b6ff9 100644
--- a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
+++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
@@ -13,7 +13,6 @@ import DateRelativeWidget from "./widgets/DateRelativeWidget";
import DateMonthYearWidget from "./widgets/DateMonthYearWidget";
import DateQuarterYearWidget from "./widgets/DateQuarterYearWidget";
import DateAllOptionsWidget from "./widgets/DateAllOptionsWidget";
-import CategoryWidget from "./widgets/CategoryWidget";
import TextWidget from "./widgets/TextWidget";
import ParameterFieldWidget from "./widgets/ParameterFieldWidget";
@@ -84,22 +83,20 @@ export default class ParameterValueWidget extends Component {
className: "",
};
- // this method assumes the parameter is associated with only one field
- getSingleField() {
- const { parameter, metadata } = this.props;
- return parameter.field_id != null
- ? metadata.fields[parameter.field_id]
- : null;
+ getFields() {
+ const { metadata } = this.props;
+ if (!metadata) {
+ return [];
+ }
+ return this.fieldIds(this.props).map(id => metadata.field(id));
}
getWidget() {
- const { parameter, values } = this.props;
+ const { parameter } = this.props;
if (DATE_WIDGETS[parameter.type]) {
return DATE_WIDGETS[parameter.type];
- } else if (this.getSingleField()) {
+ } else if (this.getFields().length > 0) {
return ParameterFieldWidget;
- } else if (values && values.length > 0) {
- return CategoryWidget;
} else {
return TextWidget;
}
@@ -115,7 +112,7 @@ export default class ParameterValueWidget extends Component {
}
}
- fieldIds({ parameter: { field_id, field_ids = [] } }) {
+ fieldIds({ parameter: { field_ids = [], field_id } }) {
return field_id ? [field_id] : field_ids;
}
@@ -231,7 +228,7 @@ export default class ParameterValueWidget extends Component {
placeholder={placeholder}
value={value}
values={values}
- field={this.getSingleField()}
+ fields={this.getFields()}
setValue={setValue}
isEditing={isEditing}
commitImmediately={commitImmediately}
diff --git a/frontend/src/metabase/parameters/components/widgets/CategoryWidget.jsx b/frontend/src/metabase/parameters/components/widgets/CategoryWidget.jsx
deleted file mode 100644
index da86242943b..00000000000
--- a/frontend/src/metabase/parameters/components/widgets/CategoryWidget.jsx
+++ /dev/null
@@ -1,113 +0,0 @@
-/* @flow */
-/* eslint "react/prop-types": "warn" */
-
-import React, { Component } from "react";
-import PropTypes from "prop-types";
-import { t, ngettext, msgid } from "ttag";
-
-import { createMultiwordSearchRegex } from "metabase/lib/string";
-import { getHumanReadableValue } from "metabase/lib/query/field";
-
-import SelectPicker from "../../../query_builder/components/filters/pickers/SelectPicker";
-
-type Props = {
- value: any,
- values: any[],
- setValue: () => void,
- onClose: () => void,
-};
-type State = {
- searchText: string,
- searchRegex: ?RegExp,
- selectedValues: Array<string>,
-};
-
-export default class CategoryWidget extends Component {
- props: Props;
- state: State;
-
- constructor(props: Props) {
- super(props);
-
- this.state = {
- searchText: "",
- searchRegex: null,
- selectedValues: Array.isArray(props.value) ? props.value : [props.value],
- };
- }
-
- static propTypes = {
- value: PropTypes.any,
- values: PropTypes.array.isRequired,
- setValue: PropTypes.func.isRequired,
- onClose: PropTypes.func.isRequired,
- };
-
- updateSearchText = (value: string) => {
- let regex = null;
-
- if (value) {
- regex = createMultiwordSearchRegex(value);
- }
-
- this.setState({
- searchText: value,
- searchRegex: regex,
- });
- };
-
- static format(values, fieldValues) {
- if (Array.isArray(values) && values.length > 1) {
- const n = values.length;
- return ngettext(msgid`${n} selection`, `${n} selections`, n);
- } else {
- return getHumanReadableValue(values, fieldValues);
- }
- }
-
- getOptions() {
- return this.props.values.slice().map(value => {
- return {
- name: value[0],
- key: value[0],
- };
- });
- }
-
- commitValues = (values: ?Array<string>) => {
- if (values && values.length === 0) {
- values = null;
- }
- this.props.setValue(values);
- this.props.onClose();
- };
-
- onSelectedValuesChange = (values: Array<string>) => {
- this.setState({ selectedValues: values });
- };
-
- render() {
- const options = this.getOptions();
- const selectedValues = this.state.selectedValues;
-
- return (
- <div style={{ minWidth: 182 }}>
- <SelectPicker
- options={options}
- values={(selectedValues: Array<string>)}
- onValuesChange={this.onSelectedValuesChange}
- multi={true}
- />
- <div className="p1">
- <button
- data-ui-tag="add-category-filter"
- className="Button Button--purple full"
- onClick={() => this.commitValues(this.state.selectedValues)}
- >
- {t`Done`}
- </button>
- </div>
- </div>
- );
- }
-}
diff --git a/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx b/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx
index 5c19885afcc..876bf51d5c2 100644
--- a/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx
+++ b/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx
@@ -8,7 +8,7 @@ import { t, ngettext, msgid } from "ttag";
import FieldValuesWidget from "metabase/components/FieldValuesWidget";
import Popover from "metabase/components/Popover";
import Button from "metabase/components/Button";
-import RemappedValue from "metabase/containers/RemappedValue";
+import Value from "metabase/components/Value";
import Field from "metabase-lib/lib/metadata/Field";
@@ -18,7 +18,7 @@ type Props = {
isEditing: boolean,
- field: Field,
+ fields: Field[],
parentFocusChanged: boolean => void,
};
@@ -51,13 +51,21 @@ export default class ParameterFieldWidget extends Component<*, Props, State> {
static noPopover = true;
- static format(value, field) {
+ static format(value, fields) {
value = normalizeValue(value);
if (value.length > 1) {
const n = value.length;
return ngettext(msgid`${n} selection`, `${n} selections`, n);
} else {
- return <RemappedValue value={value[0]} column={field} />;
+ return (
+ <Value
+ // If there are multiple fields, turn off remapping since they might
+ // be remapped to different fields.
+ remap={fields.length === 1}
+ value={value[0]}
+ column={fields[0]}
+ />
+ );
}
}
@@ -78,7 +86,7 @@ export default class ParameterFieldWidget extends Component<*, Props, State> {
}
render() {
- const { setValue, isEditing, field, parentFocusChanged } = this.props;
+ const { setValue, isEditing, fields, parentFocusChanged } = this.props;
const { isFocused } = this.state;
const savedValue = normalizeValue(this.props.value);
@@ -107,7 +115,7 @@ export default class ParameterFieldWidget extends Component<*, Props, State> {
onClick={() => focusChanged(true)}
>
{savedValue.length > 0 ? (
- ParameterFieldWidget.format(savedValue, field)
+ ParameterFieldWidget.format(savedValue, fields)
) : (
<span>{placeholder}</span>
)}
@@ -131,8 +139,7 @@ export default class ParameterFieldWidget extends Component<*, Props, State> {
this.setState({ value });
}}
placeholder={placeholder}
- field={field}
- searchField={field.parameterSearchField()}
+ fields={fields}
multi
autoFocus
color="brand"
diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx
index 8f8bff2efcc..0244214df3c 100644
--- a/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx
+++ b/frontend/src/metabase/query_builder/components/filters/pickers/DefaultPicker.jsx
@@ -81,8 +81,8 @@ export default function DefaultPicker({
onChange={onValuesChange}
multi={operator.multi}
placeholder={placeholder}
- field={underlyingField}
- searchField={underlyingField.filterSearchField()}
+ fields={underlyingField ? [underlyingField] : []}
+ disablePKRemappingForSearch={true}
autoFocus={index === 0}
alwaysShowOptions={operator.fields.length === 1}
formatOptions={getFilterArgumentFormatOptions(operator, index)}
diff --git a/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js b/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js
index 413a2984c25..8bfd38f333a 100644
--- a/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js
+++ b/frontend/test/metabase/components/FieldValuesWidget.unit.spec.js
@@ -1,7 +1,7 @@
import React from "react";
import { mount } from "enzyme";
-import { ORDERS, PRODUCTS } from "__support__/sample_dataset_fixture";
+import { ORDERS, PRODUCTS, PEOPLE } from "__support__/sample_dataset_fixture";
import { FieldValuesWidget } from "metabase/components/FieldValuesWidget";
import TokenField from "metabase/components/TokenField";
@@ -23,9 +23,7 @@ describe("FieldValuesWidget", () => {
describe("category field", () => {
describe("has_field_values = none", () => {
const props = {
- field: mock(PRODUCTS.CATEGORY, {
- has_field_values: "none",
- }),
+ fields: [mock(PRODUCTS.CATEGORY, { has_field_values: "none" })],
};
it("should not call fetchFieldValues", () => {
const fetchFieldValues = jest.fn();
@@ -41,7 +39,7 @@ describe("FieldValuesWidget", () => {
});
describe("has_field_values = list", () => {
const props = {
- field: PRODUCTS.CATEGORY,
+ fields: [PRODUCTS.CATEGORY],
};
it("should call fetchFieldValues", () => {
const fetchFieldValues = jest.fn();
@@ -57,10 +55,7 @@ describe("FieldValuesWidget", () => {
});
describe("has_field_values = search", () => {
const props = {
- field: mock(PRODUCTS.CATEGORY, {
- has_field_values: "search",
- }),
- searchField: PRODUCTS.CATEGORY,
+ fields: [mock(PRODUCTS.CATEGORY, { has_field_values: "search" })],
};
it("should not call fetchFieldValues", () => {
const fetchFieldValues = jest.fn();
@@ -79,9 +74,7 @@ describe("FieldValuesWidget", () => {
describe("has_field_values = none", () => {
it("should have 'Enter an ID' as the placeholder text", () => {
const component = mountFieldValuesWidget({
- field: mock(ORDERS.PRODUCT_ID, {
- has_field_values: "none",
- }),
+ fields: [mock(ORDERS.PRODUCT_ID, { has_field_values: "none" })],
});
expect(component.find(TokenField).props().placeholder).toEqual(
"Enter an ID",
@@ -91,10 +84,12 @@ describe("FieldValuesWidget", () => {
describe("has_field_values = list", () => {
it("should have 'Search the list' as the placeholder text", () => {
const component = mountFieldValuesWidget({
- field: mock(ORDERS.PRODUCT_ID, {
- has_field_values: "list",
- values: [[1234]],
- }),
+ fields: [
+ mock(ORDERS.PRODUCT_ID, {
+ has_field_values: "list",
+ values: [[1234]],
+ }),
+ ],
});
expect(component.find(TokenField).props().placeholder).toEqual(
"Search the list",
@@ -104,28 +99,65 @@ describe("FieldValuesWidget", () => {
describe("has_field_values = search", () => {
it("should have 'Search by Category or enter an ID' as the placeholder text", () => {
const component = mountFieldValuesWidget({
- field: mock(ORDERS.PRODUCT_ID, {
- has_field_values: "search",
- }),
- searchField: PRODUCTS.CATEGORY,
+ fields: [
+ mock(ORDERS.PRODUCT_ID, {
+ has_field_values: "search",
+ remappedField: () => PRODUCTS.CATEGORY,
+ }),
+ ],
});
expect(component.find(TokenField).props().placeholder).toEqual(
"Search by Category or enter an ID",
);
});
it("should not duplicate 'ID' in placeholder when ID itself is searchable", () => {
- const field = mock(ORDERS.PRODUCT_ID, {
- base_type: "type/Text",
- has_field_values: "search",
- });
- const component = mountFieldValuesWidget({
- field: field,
- searchField: field,
- });
+ const fields = [
+ mock(ORDERS.PRODUCT_ID, {
+ base_type: "type/Text",
+ has_field_values: "search",
+ }),
+ ];
+ const component = mountFieldValuesWidget({ fields });
expect(component.find(TokenField).props().placeholder).toEqual(
"Search by Product",
);
});
});
});
+ describe("multiple fields", () => {
+ it("list multiple fields together", () => {
+ const fields = [
+ mock(PEOPLE.SOURCE, { has_field_values: "list" }),
+ mock(PEOPLE.STATE, { has_field_values: "list" }),
+ ];
+ const component = mountFieldValuesWidget({ fields });
+ const { placeholder, options } = component.find(TokenField).props();
+ expect(placeholder).toEqual("Search the list");
+ const optionValues = options.map(([value]) => value);
+ expect(optionValues).toContain("AZ");
+ expect(optionValues).toContain("Facebook");
+ });
+
+ it("search if any field is a search", () => {
+ const fields = [
+ mock(PEOPLE.SOURCE, { has_field_values: "search" }),
+ mock(PEOPLE.STATE, { has_field_values: "list" }),
+ ];
+ const component = mountFieldValuesWidget({ fields });
+ const { placeholder, options } = component.find(TokenField).props();
+ expect(placeholder).toEqual("Search");
+ expect(options.length).toBe(0);
+ });
+
+ it("don't list any values if any is set to 'plain input box'", () => {
+ const fields = [
+ mock(PEOPLE.SOURCE, { has_field_values: "none" }),
+ mock(PEOPLE.STATE, { has_field_values: "list" }),
+ ];
+ const component = mountFieldValuesWidget({ fields });
+ const { placeholder, options } = component.find(TokenField).props();
+ expect(placeholder).toEqual("Enter some text");
+ expect(options.length).toBe(0);
+ });
+ });
});
diff --git a/frontend/test/metabase/parameters/components/widgets/CategoryWidget.e2e.spec.js b/frontend/test/metabase/parameters/components/widgets/CategoryWidget.e2e.spec.js
deleted file mode 100644
index 81275ddb2b2..00000000000
--- a/frontend/test/metabase/parameters/components/widgets/CategoryWidget.e2e.spec.js
+++ /dev/null
@@ -1,84 +0,0 @@
-import "__support__/e2e";
-
-import React from "react";
-
-import CategoryWidget from "metabase/parameters/components/widgets/CategoryWidget";
-
-import { mount } from "enzyme";
-import { click, clickButton } from "__support__/enzyme";
-
-const VALUES = [["First"], ["Second"], ["Third"]];
-
-const ON_SET_VALUE = jest.fn();
-
-function renderCategoryWidget(props) {
- return mount(
- <CategoryWidget
- values={VALUES}
- setValue={ON_SET_VALUE}
- onClose={() => {}}
- {...props}
- />,
- );
-}
-
-describe("CategoryWidget", () => {
- describe("with a selected value", () => {
- it("should render with selected value checked", () => {
- const categoryWidget = renderCategoryWidget({ value: VALUES[0] });
- expect(categoryWidget.find(".Icon-check").length).toEqual(1);
- categoryWidget
- .find("label")
- .findWhere(label => label.text().match(/First/))
- .find(".Icon-check")
- .exists();
- });
- });
-
- describe("without a selected value", () => {
- it("should render with selected value checked", () => {
- const categoryWidget = renderCategoryWidget({ value: [] });
- expect(categoryWidget.find(".Icon-check").length).toEqual(0);
- });
- });
-
- describe("selecting values", () => {
- it("should mark the values as selected", () => {
- const categoryWidget = renderCategoryWidget({ value: [] });
- // Check option 1
- click(categoryWidget.find("label").at(0));
- expect(categoryWidget.find(".Icon-check").length).toEqual(1);
-
- // Check option 2
- click(categoryWidget.find("label").at(1));
- expect(categoryWidget.find(".Icon-check").length).toEqual(2);
-
- clickButton(categoryWidget.find(".Button"));
-
- expect(ON_SET_VALUE).toHaveBeenCalledWith(["First", "Second"]);
-
- // Un-check option 1
- click(categoryWidget.find("label").at(0));
- expect(categoryWidget.find(".Icon-check").length).toEqual(1);
-
- clickButton(categoryWidget.find(".Button"));
-
- expect(ON_SET_VALUE).toHaveBeenCalledWith(["Second"]);
- });
- });
-
- describe("selecting no values", () => {
- it("selected values should be null", () => {
- const categoryWidget = renderCategoryWidget({ value: [] });
- // Check option 1
- click(categoryWidget.find("label").at(0));
- clickButton(categoryWidget.find(".Button"));
- expect(ON_SET_VALUE).toHaveBeenCalledWith(["First"]);
-
- // un-check option 1
- click(categoryWidget.find("label").at(0));
- clickButton(categoryWidget.find(".Button"));
- expect(ON_SET_VALUE).toHaveBeenCalledWith(null);
- });
- });
-});
diff --git a/frontend/test/metabase/parameters/parameters.e2e.spec.js b/frontend/test/metabase/parameters/parameters.e2e.spec.js
deleted file mode 100644
index fb4ea06083b..00000000000
--- a/frontend/test/metabase/parameters/parameters.e2e.spec.js
+++ /dev/null
@@ -1,511 +0,0 @@
-jest.mock("metabase/query_builder/components/NativeQueryEditor");
-
-import { mount } from "enzyme";
-
-import {
- createSavedQuestion,
- createDashboard,
- createTestStore,
- useSharedAdminLogin,
- logout,
- waitForRequestToComplete,
- waitForAllRequestsToComplete,
- cleanup,
- eventually,
-} from "__support__/e2e";
-
-import jwt from "jsonwebtoken";
-
-import { FETCH_DASHBOARD } from "metabase/dashboard/dashboard";
-import { fetchTableMetadata } from "metabase/redux/metadata";
-import { getMetadata } from "metabase/selectors/metadata";
-
-import ParameterWidget from "metabase/parameters/components/ParameterWidget";
-import FieldValuesWidget from "metabase/components/FieldValuesWidget";
-import ParameterFieldWidget from "metabase/parameters/components/widgets/ParameterFieldWidget";
-import TokenField from "metabase/components/TokenField";
-
-import * as Urls from "metabase/lib/urls";
-import Question from "metabase-lib/lib/Question";
-
-import {
- CardApi,
- DashboardApi,
- SettingsApi,
- MetabaseApi,
-} from "metabase/services";
-
-const ORDER_USER_ID_FIELD_ID = 7;
-const PEOPLE_ID_FIELD_ID = 13;
-const PEOPLE_NAME_FIELD_ID = 16;
-const PEOPLE_SOURCE_FIELD_ID = 18;
-
-const METABASE_SECRET_KEY =
- "24134bd93e081773fb178e8e1abb4e8a973822f7e19c872bd92c8d5a122ef63f";
-
-describe("parameters", () => {
- let question, dashboard;
-
- beforeAll(async () => {
- useSharedAdminLogin();
-
- // enable public sharing
- await SettingsApi.put({ key: "enable-public-sharing", value: true });
- cleanup.fn(() =>
- SettingsApi.put({ key: "enable-public-sharing", value: false }),
- );
-
- await SettingsApi.put({ key: "enable-embedding", value: true });
- cleanup.fn(() =>
- SettingsApi.put({ key: "enable-embedding", value: false }),
- );
-
- await SettingsApi.put({
- key: "embedding-secret-key",
- value: METABASE_SECRET_KEY,
- });
-
- await MetabaseApi.field_dimension_update({
- fieldId: ORDER_USER_ID_FIELD_ID,
- type: "external",
- name: "User ID",
- human_readable_field_id: PEOPLE_NAME_FIELD_ID,
- });
- cleanup.fn(() =>
- MetabaseApi.field_dimension_delete({
- fieldId: ORDER_USER_ID_FIELD_ID,
- }),
- );
-
- // set each of these fields to have "has_field_values" = "search"
- for (const fieldId of [
- ORDER_USER_ID_FIELD_ID,
- PEOPLE_ID_FIELD_ID,
- PEOPLE_NAME_FIELD_ID,
- ]) {
- const field = await MetabaseApi.field_get({
- fieldId: fieldId,
- });
- await MetabaseApi.field_update({
- id: fieldId,
- has_field_values: "search",
- });
- cleanup.fn(() => MetabaseApi.field_update(field));
- }
-
- const store = await createTestStore();
- await store.dispatch(fetchTableMetadata(1));
- const metadata = getMetadata(store.getState());
-
- const unsavedQuestion = Question.create({
- databaseId: 1,
- metadata,
- })
- .setDatasetQuery({
- type: "native",
- database: 1,
- native: {
- query:
- "SELECT COUNT(*) FROM people WHERE {{id}} AND {{name}} AND {{source}} /* AND {{user_id}} */",
- "template-tags": {
- id: {
- id: "1",
- name: "id",
- "display-name": "ID",
- type: "dimension",
- dimension: ["field-id", PEOPLE_ID_FIELD_ID],
- "widget-type": "id",
- },
- name: {
- id: "2",
- name: "name",
- "display-name": "Name",
- type: "dimension",
- dimension: ["field-id", PEOPLE_NAME_FIELD_ID],
- "widget-type": "category",
- },
- source: {
- id: "3",
- name: "source",
- "display-name": "Source",
- type: "dimension",
- dimension: ["field-id", PEOPLE_SOURCE_FIELD_ID],
- "widget-type": "category",
- },
- user_id: {
- id: "4",
- name: "user_id",
- "display-name": "User",
- type: "dimension",
- dimension: ["field-id", ORDER_USER_ID_FIELD_ID],
- "widget-type": "id",
- },
- },
- },
- parameters: [],
- })
- .setDisplay("scalar")
- .setDisplayName("Test Question");
- question = await createSavedQuestion(unsavedQuestion);
- cleanup.fn(() =>
- CardApi.update({
- id: question.id(),
- archived: true,
- }),
- );
-
- // create a dashboard
- dashboard = await createDashboard({
- name: "Test Dashboard",
- description: null,
- parameters: [
- { name: "ID", slug: "id", id: "1", type: "id" },
- { name: "Name", slug: "name", id: "2", type: "category" },
- { name: "Source", slug: "source", id: "3", type: "category" },
- { name: "User", slug: "user_id", id: "4", type: "id" },
- ],
- });
- cleanup.fn(() =>
- DashboardApi.update({
- id: dashboard.id,
- archived: true,
- }),
- );
-
- const dashcard = await DashboardApi.addcard({
- dashId: dashboard.id,
- cardId: question.id(),
- });
- await DashboardApi.reposition_cards({
- dashId: dashboard.id,
- cards: [
- {
- id: dashcard.id,
- card_id: question.id(),
- row: 0,
- col: 0,
- sizeX: 4,
- sizeY: 4,
- series: [],
- visualization_settings: {},
- parameter_mappings: [
- {
- parameter_id: "1",
- card_id: question.id(),
- target: ["dimension", ["template-tag", "id"]],
- },
- {
- parameter_id: "2",
- card_id: question.id(),
- target: ["dimension", ["template-tag", "name"]],
- },
- {
- parameter_id: "3",
- card_id: question.id(),
- target: ["dimension", ["template-tag", "source"]],
- },
- {
- parameter_id: "4",
- card_id: question.id(),
- target: ["dimension", ["template-tag", "user_id"]],
- },
- ],
- },
- ],
- });
- });
-
- describe("private questions", () => {
- let app, store;
- it("should be possible to view a private question", async () => {
- useSharedAdminLogin();
-
- store = await createTestStore();
- store.pushPath(Urls.question(question.id()) + "?id=1");
- app = mount(store.getAppContainer());
-
- await Promise.all([
- waitForRequestToComplete("GET", /^\/api\/database.*include_tables/),
- waitForRequestToComplete("GET", /^\/api\/card\/\d+/),
- ]);
- expect(app.find("ViewHeading").text()).toEqual("Test Question");
-
- // wait for the query to load
- await waitForRequestToComplete("POST", /^\/api\/card\/\d+\/query/);
- });
- sharedParametersTests(() => ({ app, store }));
- });
-
- describe("public questions", () => {
- let app, store;
- it("should be possible to view a public question", async () => {
- useSharedAdminLogin();
- const publicQuestion = await CardApi.createPublicLink({
- id: question.id(),
- });
-
- logout();
-
- store = await createTestStore({ publicApp: true });
- store.pushPath(Urls.publicQuestion(publicQuestion.uuid) + "?id=1");
- app = mount(store.getAppContainer());
-
- await waitForRequestToComplete("GET", /^\/api\/[^\/]*\/card/);
- expect(app.find(".EmbedFrame-header .h4").text()).toEqual(
- "Test Question",
- );
-
- // wait for the query to load
- await waitForRequestToComplete(
- "GET",
- /^\/api\/public\/card\/[^\/]+\/query/,
- );
- });
- sharedParametersTests(() => ({ app, store }));
- });
-
- describe("embed questions", () => {
- let app, store;
- it("should be possible to view a embedded question", async () => {
- useSharedAdminLogin();
- await CardApi.update({
- id: question.id(),
- embedding_params: {
- id: "enabled",
- name: "enabled",
- source: "enabled",
- user_id: "enabled",
- },
- enable_embedding: true,
- });
-
- logout();
-
- const token = jwt.sign(
- {
- resource: { question: question.id() },
- params: {},
- },
- METABASE_SECRET_KEY,
- );
-
- store = await createTestStore({ embedApp: true });
- store.pushPath(Urls.embedCard(token) + "?id=1");
- app = mount(store.getAppContainer());
-
- await waitForRequestToComplete("GET", /\/card\/[^\/]+/);
-
- expect(app.find(".EmbedFrame-header .h4").text()).toEqual(
- "Test Question",
- );
-
- // wait for the query to load
- await waitForRequestToComplete(
- "GET",
- /^\/api\/embed\/card\/[^\/]+\/query/,
- );
- });
- sharedParametersTests(() => ({ app, store }));
- });
-
- describe("private dashboards", () => {
- let app, store;
- it("should be possible to view a private dashboard", async () => {
- useSharedAdminLogin();
-
- store = await createTestStore();
- store.pushPath(Urls.dashboard(dashboard.id) + "?id=1");
- app = mount(store.getAppContainer());
-
- await store.waitForActions([FETCH_DASHBOARD]);
- expect(app.find(".DashboardHeader .Entity .h2").text()).toEqual(
- "Test Dashboard",
- );
-
- // wait for the query to load
- await waitForRequestToComplete("POST", /^\/api\/card\/[^\/]+\/query/);
-
- // wait for required field metadata to load
- await waitForRequestToComplete("GET", /^\/api\/field\/[^\/]+/);
- });
- sharedParametersTests(() => ({ app, store }));
- });
-
- describe("public dashboards", () => {
- let app, store;
- it("should be possible to view a public dashboard", async () => {
- useSharedAdminLogin();
- const publicDash = await DashboardApi.createPublicLink({
- id: dashboard.id,
- });
-
- logout();
-
- store = await createTestStore({ publicApp: true });
- store.pushPath(Urls.publicDashboard(publicDash.uuid) + "?id=1");
- app = mount(store.getAppContainer());
-
- await store.waitForActions([FETCH_DASHBOARD]);
- expect(app.find(".EmbedFrame-header .h4").text()).toEqual(
- "Test Dashboard",
- );
-
- // wait for the query to load
- await waitForRequestToComplete(
- "GET",
- /^\/api\/public\/dashboard\/[^\/]+\/card\/[^\/]+/,
- );
- });
- sharedParametersTests(() => ({ app, store }));
- });
-
- describe("embed dashboards", () => {
- let app, store;
- it("should be possible to view a embed dashboard", async () => {
- useSharedAdminLogin();
- await DashboardApi.update({
- id: dashboard.id,
- embedding_params: {
- id: "enabled",
- name: "enabled",
- source: "enabled",
- user_id: "enabled",
- },
- enable_embedding: true,
- });
-
- logout();
-
- const token = jwt.sign(
- {
- resource: { dashboard: dashboard.id },
- params: {},
- },
- METABASE_SECRET_KEY,
- );
-
- store = await createTestStore({ embedApp: true });
- store.pushPath(Urls.embedDashboard(token) + "?id=1");
- app = mount(store.getAppContainer());
-
- await store.waitForActions([FETCH_DASHBOARD]);
-
- expect(app.find(".EmbedFrame-header .h4").text()).toEqual(
- "Test Dashboard",
- );
-
- // wait for the query to load
- await waitForRequestToComplete(
- "GET",
- /^\/api\/embed\/dashboard\/[^\/]+\/dashcard\/\d+\/card\/\d+/,
- );
- });
- sharedParametersTests(() => ({ app, store }));
- });
-
- afterAll(cleanup);
-});
-
-async function sharedParametersTests(getAppAndStore) {
- let app;
- beforeEach(() => {
- const info = getAppAndStore();
- app = info.app;
- });
-
- it("should have 4 ParameterFieldWidgets", async () => {
- await waitForAllRequestsToComplete();
-
- expect(app.find(ParameterWidget).length).toEqual(4);
- expect(app.find(ParameterFieldWidget).length).toEqual(4);
- });
-
- it("open 4 FieldValuesWidgets", async () => {
- // click each parameter to open the widget
- app.find(ParameterFieldWidget).map(widget => widget.simulate("click"));
-
- const widgets = app.find(FieldValuesWidget);
- expect(widgets.length).toEqual(4);
- });
-
- // it("should have the correct field and searchField", () => {
- // const widgets = app.find(FieldValuesWidget);
- // expect(
- // widgets.map(widget => {
- // const { field, searchField } = widget.props();
- // return [field && field.id, searchField && searchField.id];
- // }),
- // ).toEqual([
- // [PEOPLE_ID_FIELD_ID, PEOPLE_NAME_FIELD_ID],
- // [PEOPLE_NAME_FIELD_ID, PEOPLE_NAME_FIELD_ID],
- // [PEOPLE_SOURCE_FIELD_ID, PEOPLE_SOURCE_FIELD_ID],
- // [ORDER_USER_ID_FIELD_ID, PEOPLE_NAME_FIELD_ID],
- // ]);
- // });
-
- it("should have the correct values", async () => {
- await eventually(() => {
- const widgets = app.find(FieldValuesWidget);
- const values = widgets.map(
- widget =>
- widget
- .find("ul") // first ul is options
- .at(0)
- .find("li")
- .map(li => li.text())
- .slice(0, -1), // the last item is the input, remove it
- );
- expect(values).toEqual([
- ["Hudson Borer - 1"], // remapped value
- [],
- [],
- [],
- ]);
- });
- });
-
- it("should have the correct placeholders", () => {
- const widgets = app.find(FieldValuesWidget);
- const placeholders = widgets.map(
- widget => widget.find(TokenField).props().placeholder,
- );
- expect(placeholders).toEqual([
- "Search by Name or enter an ID",
- "Search by Name",
- "Search the list",
- "Search by Name or enter an ID",
- ]);
- });
-
- it("should allow searching PEOPLE.ID by PEOPLE.NAME", async () => {
- const widget = app.find(FieldValuesWidget).at(0);
- // tests `search` endpoint
- expect(widget.find("li").length).toEqual(1 + 1);
- widget.find("input").simulate("change", { target: { value: "Aly" } });
- await waitForRequestToComplete("GET", /\/field\/.*\/search/);
- expect(widget.find("li").length).toEqual(1 + 1 + 4);
- });
- it("should allow searching PEOPLE.NAME by PEOPLE.NAME", async () => {
- const widget = app.find(FieldValuesWidget).at(1);
- // tests `search` endpoint
- expect(widget.find("li").length).toEqual(1);
- widget.find("input").simulate("change", { target: { value: "Aly" } });
- await waitForRequestToComplete("GET", /\/field\/.*\/search/);
- expect(widget.find("li").length).toEqual(1 + 4);
- });
- it("should show values for PEOPLE.SOURCE", async () => {
- const widget = app.find(FieldValuesWidget).at(2);
- // tests `values` endpoint
- // NOTE: no need for waitForRequestToComplete because it was previously loaded?
- // await waitForRequestToComplete("GET", /\/field\/.*\/values/);
- expect(widget.find("li").length).toEqual(1 + 5); // 5 options + 1 for the input
- });
- it("should allow searching ORDER.USER_ID by PEOPLE.NAME", async () => {
- const widget = app.find(FieldValuesWidget).at(3);
- // tests `search` endpoint
- expect(widget.find("li").length).toEqual(1);
- widget.find("input").simulate("change", { target: { value: "Aly" } });
- await waitForRequestToComplete("GET", /\/field\/.*\/search/);
- expect(widget.find("li").length).toEqual(1 + 4);
- });
-}
--
GitLab