diff --git a/bin/i18n/src/i18n/common.clj b/bin/i18n/src/i18n/common.clj index 28d0c123e0716..379df14445aed 100644 --- a/bin/i18n/src/i18n/common.clj +++ b/bin/i18n/src/i18n/common.clj @@ -21,7 +21,7 @@ (defn- catalog ^Catalog [locale] (let [parser (PoParser.)] - (.parseCatalog parser (io/file (locale-source-po-filename "es"))))) + (.parseCatalog parser (io/file (locale-source-po-filename locale))))) (defn po-headers [locale] (when-let [^Message message (.locateHeader (catalog locale))] diff --git a/dev/src/dev.clj b/dev/src/dev.clj index db28f2f20a014..aad0b5e49a9e3 100644 --- a/dev/src/dev.clj +++ b/dev/src/dev.clj @@ -22,6 +22,9 @@ (comment debug-qp/keep-me) +(defn tap>-spy [x] + (doto x tap>)) + (p/import-vars [debug-qp process-query-debug]) diff --git a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj index 2e386a9acdda2..293dea75ec8b8 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj @@ -3,10 +3,10 @@ (:require [clojure.data :as diff] [clojure.java.io :as io] [clojure.test :refer [deftest is]] - [expectations :refer [expect]] [metabase-enterprise.serialization.cmd :refer [dump load]] [metabase-enterprise.serialization.test-util :as ts] - [metabase.models :refer [Card Collection Dashboard DashboardCard DashboardCardSeries Database Dependency Dimension Field FieldValues Metric Pulse PulseCard PulseChannel Segment Table User]] + [metabase.models :refer [Card Collection Dashboard DashboardCard DashboardCardSeries Database Dependency + Dimension Field FieldValues Metric Pulse PulseCard PulseChannel Segment Table User]] [metabase.test.data.users :as test-users] [metabase.util :as u] [toucan.db :as db]) diff --git a/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj index c2cbb6498ccd3..08890716e931a 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/names_test.clj @@ -1,66 +1,39 @@ (ns metabase-enterprise.serialization.names-test - (:require [expectations :refer :all] - [metabase-enterprise.serialization.names :as names :refer :all] + (:require [clojure.test :refer :all] + [metabase-enterprise.serialization.names :as names] [metabase-enterprise.serialization.test-util :as ts] [metabase.models :refer [Card Collection Dashboard Database Field Metric Segment Table]] [metabase.util :as u])) -(expect - (= (safe-name {:name "foo"}) "foo")) -(expect - (= (safe-name {:name "foo/bar baz"}) "foo%2Fbar baz")) - -(expect - (= (unescape-name "foo") "foo")) -(expect - (= (unescape-name "foo%2Fbar baz") "foo/bar baz")) - -(expect - (let [n "foo/bar baz"] - (= (-> {:name n} safe-name unescape-name (= n))))) - -(defn- test-fully-qualified-name-roundtrip - [entity] - (let [context (fully-qualified-name->context (fully-qualified-name entity))] - (= (u/get-id entity) ((some-fn :field :metric :segment :card :dashboard :collection :table :database) context)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Card card-id-root)))) -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Card card-id)))) -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Card card-id-nested)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Table table-id)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Field category-field-id)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Metric metric-id)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Segment segment-id)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Collection collection-id)))) -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Collection collection-id-nested)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Dashboard dashboard-id)))) - -(expect - (ts/with-world - (test-fully-qualified-name-roundtrip (Database db-id)))) +(deftest safe-name-test + (are [s expected] (= (names/safe-name {:name s}) expected) + "foo" "foo" + "foo/bar baz" "foo%2Fbar baz")) + +(deftest unescape-name-test + (are [s expected] (= expected + (names/unescape-name s)) + "foo" "foo" + "foo%2Fbar baz" "foo/bar baz")) + +(deftest safe-name-unescape-name-test + (is (= "foo/bar baz" + (-> {:name "foo/bar baz"} names/safe-name names/unescape-name)))) + +(deftest roundtrip-test + (ts/with-world + (doseq [object [(Card card-id-root) + (Card card-id) + (Card card-id-nested) + (Table table-id) + (Field category-field-id) + (Metric metric-id) + (Segment segment-id) + (Collection collection-id) + (Collection collection-id-nested) + (Dashboard dashboard-id) + (Database db-id)]] + (testing (class object) + (let [context (names/fully-qualified-name->context (names/fully-qualified-name object))] + (is (= (u/the-id object) + ((some-fn :field :metric :segment :card :dashboard :collection :table :database) context)))))))) diff --git a/frontend/src/metabase-lib/lib/Question.js b/frontend/src/metabase-lib/lib/Question.js index be567e7390a18..0cb5c8a8e6025 100644 --- a/frontend/src/metabase-lib/lib/Question.js +++ b/frontend/src/metabase-lib/lib/Question.js @@ -31,7 +31,10 @@ import * as Card_DEPRECATED from "metabase/lib/card"; import * as Urls from "metabase/lib/urls"; import { syncTableColumnsToQuery } from "metabase/lib/dataset"; import { getParametersWithExtras, isTransientId } from "metabase/meta/Card"; -import { parameterToMBQLFilter } from "metabase/meta/Parameter"; +import { + parameterToMBQLFilter, + mapUIParameterToQueryParameter, +} from "metabase/meta/Parameter"; import { aggregate, breakout, @@ -823,7 +826,10 @@ export default class Question { // include only parameters that have a value applied .filter(param => _.has(param, "value")) // only the superset of parameters object that API expects - .map(param => _.pick(param, "type", "target", "value")); + .map(param => _.pick(param, "type", "target", "value")) + .map(({ type, value, target }) => { + return mapUIParameterToQueryParameter(type, value, target); + }); if (canUseCardApiEndpoint) { const queryParams = { diff --git a/frontend/src/metabase/components/CollapseSection.info.js b/frontend/src/metabase/components/CollapseSection.info.js new file mode 100644 index 0000000000000..e44df34673cde --- /dev/null +++ b/frontend/src/metabase/components/CollapseSection.info.js @@ -0,0 +1,63 @@ +import React from "react"; +import CollapseSection from "metabase/components/CollapseSection"; +import Icon from "metabase/components/Icon"; + +export const component = CollapseSection; +export const category = "layout"; +export const description = ` +A collapsible section with a clickable header. +`; + +export const examples = { + "Collapsed by default": ( + + foo foo foo foo foo foo foo foo + + ), + "Settable collpased/expanded initial state": ( + + foo foo foo foo foo + + ), + "Components in header": ( + + + Component header + + } + > + foo foo foo foo foo + + ), + "Header and body classes": ( + +
+ Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus neque + tellus, mattis ut felis non, tempus mollis lacus. Vivamus nulla massa, + accumsan non ligula eu, dapibus volutpat libero. Mauris sollicitudin + dolor et ipsum fringilla auctor. Praesent et diam non nisi consequat + ornare. Aenean et risus vel dolor maximus dapibus a id massa. Nam + finibus quis libero eu finibus. Sed vehicula ac enim pellentesque + luctus. Phasellus vehicula et ipsum porttitor mollis. Fusce blandit + lacus a elit pretium, vestibulum porta nisi vehicula. Aliquam vel ligula + enim. Orci varius natoque penatibus et magnis dis parturient montes, + nascetur ridiculus mus. Pellentesque eget porta mi. Duis et lectus eget + dolor convallis mollis. Sed commodo nec urna eget egestas. +
+
+ Mauris in ante sit amet ipsum tempus consequat. Curabitur auctor massa + vitae dui auctor scelerisque. Donec in leo a libero commodo sodales. + Integer egestas lacinia elit, vitae cursus sem mollis ut. Proin ut + dapibus metus, vel accumsan justo. Pellentesque eget finibus elit, ut + commodo felis. Ut non lacinia metus. Maecenas eget bibendum nisl. +
+
+ ), +}; diff --git a/frontend/src/metabase/components/CollapseSection.jsx b/frontend/src/metabase/components/CollapseSection.jsx new file mode 100644 index 0000000000000..f5cafd214ad57 --- /dev/null +++ b/frontend/src/metabase/components/CollapseSection.jsx @@ -0,0 +1,70 @@ +/* eslint "react/prop-types": 2 */ + +import React, { useState } from "react"; +import PropTypes from "prop-types"; +import cx from "classnames"; + +import Icon from "metabase/components/Icon"; + +function CollapseSection({ + children, + className, + header, + headerClass, + bodyClass, + initialState = "collapsed", +}) { + const [isExpanded, setIsExpanded] = useState(initialState === "expanded"); + + return ( +
+
setIsExpanded(isExpanded => !isExpanded)} + onKeyDown={e => + e.key === "Enter" && setIsExpanded(isExpanded => !isExpanded) + } + > + + + {header} + +
+
+ {isExpanded && ( +
+ {children} +
+ )} +
+
+ ); +} + +CollapseSection.propTypes = { + children: PropTypes.node, + className: PropTypes.string, + header: PropTypes.node, + headerClass: PropTypes.string, + bodyClass: PropTypes.string, + initialState: PropTypes.oneOf(["expanded", "collapsed"]), +}; + +export default CollapseSection; diff --git a/frontend/src/metabase/dashboard/components/ParametersPopover.jsx b/frontend/src/metabase/dashboard/components/ParametersPopover.jsx index b0740a46d2ee6..fcfa5945478c2 100644 --- a/frontend/src/metabase/dashboard/components/ParametersPopover.jsx +++ b/frontend/src/metabase/dashboard/components/ParametersPopover.jsx @@ -1,6 +1,9 @@ import React, { Component } from "react"; import { t } from "ttag"; import { PARAMETER_SECTIONS } from "metabase/meta/Dashboard"; +import Icon from "metabase/components/Icon"; +import { getParameterIconName } from "metabase/meta/Parameter"; +import styled from "styled-components"; import type { Parameter, @@ -11,6 +14,10 @@ import _ from "underscore"; import type { ParameterSection } from "metabase/meta/Dashboard"; +const PopoverBody = styled.div` + max-width: 300px; +`; + export default class ParametersPopover extends Component { props: { onAddParameter: (option: ParameterOption) => Promise, @@ -68,7 +75,11 @@ export const ParameterOptionsSection = ({ onClick: () => any, }) => (
  • -
    +
    + {section.name}
    {section.description}
    @@ -82,7 +93,7 @@ export const ParameterOptionsSectionsPane = ({ sections: Array, onSelectSection: ParameterSection => any, }) => ( -
    +

    {t`What do you want to filter?`}

      {sections.map(section => ( @@ -92,7 +103,7 @@ export const ParameterOptionsSectionsPane = ({ /> ))}
    -
    + ); export const ParameterOptionItem = ({ @@ -117,7 +128,7 @@ export const ParameterOptionsPane = ({ options: ?Array, onSelectOption: ParameterOption => any, }) => ( -
    +

    {t`What kind of filter?`}

      {options && @@ -128,5 +139,5 @@ export const ParameterOptionsPane = ({ /> ))}
    -
    + ); diff --git a/frontend/src/metabase/dashboard/selectors.js b/frontend/src/metabase/dashboard/selectors.js index 3dd089abc22a1..9cf24992611ec 100644 --- a/frontend/src/metabase/dashboard/selectors.js +++ b/frontend/src/metabase/dashboard/selectors.js @@ -251,3 +251,15 @@ export const makeGetParameterMappingOptions = () => { ); return getParameterMappingOptions; }; + +export const getDefaultParametersById = createSelector( + [getDashboard], + dashboard => + ((dashboard && dashboard.parameters) || []).reduce((map, parameter) => { + if (parameter.default) { + map[parameter.id] = parameter.default; + } + + return map; + }, {}), +); diff --git a/frontend/src/metabase/lib/schema_metadata.js b/frontend/src/metabase/lib/schema_metadata.js index 9caf12883d77f..3e4f99ee5b884 100644 --- a/frontend/src/metabase/lib/schema_metadata.js +++ b/frontend/src/metabase/lib/schema_metadata.js @@ -441,6 +441,10 @@ const FILTER_OPERATORS_BY_TYPE_ORDERED = { { name: "!=", verboseName: t`Is not` }, { name: "is-null", verboseName: t`Is empty` }, { name: "not-null", verboseName: t`Not empty` }, + { name: "contains", verboseName: t`Contains` }, + { name: "does-not-contain", verboseName: t`Does not contain` }, + { name: "starts-with", verboseName: t`Starts with` }, + { name: "ends-with", verboseName: t`Ends with` }, ], [COORDINATE]: [ { name: "=", verboseName: t`Is` }, @@ -470,6 +474,28 @@ const MORE_VERBOSE_NAMES = { "greater than or equal to": "is greater than or equal to", }; +export function doesOperatorExist(operatorName) { + return !!FIELD_FILTER_OPERATORS[operatorName]; +} + +export function getOperatorByTypeAndName(type, name) { + const typedNamedOperator = _.findWhere( + FILTER_OPERATORS_BY_TYPE_ORDERED[type], + { + name, + }, + ); + const namedOperator = FIELD_FILTER_OPERATORS[name]; + + return ( + typedNamedOperator && { + ...typedNamedOperator, + ...namedOperator, + numFields: namedOperator.validArgumentsFilters.length, + } + ); +} + export function getFilterOperators(field, table, selected) { const type = getFieldType(field) || UNKNOWN; return FILTER_OPERATORS_BY_TYPE_ORDERED[type] @@ -721,3 +747,7 @@ export function getFilterArgumentFormatOptions(filterOperator, index) { {} ); } + +export function isEqualsOperator(operator) { + return !!operator && operator.name === "="; +} diff --git a/frontend/src/metabase/meta/Card.js b/frontend/src/metabase/meta/Card.js index 0d63454ef56e2..c996d0ca06ed8 100644 --- a/frontend/src/metabase/meta/Card.js +++ b/frontend/src/metabase/meta/Card.js @@ -2,6 +2,7 @@ import { getTemplateTagParameters, getParameterTargetFieldId, parameterToMBQLFilter, + mapUIParameterToQueryParameter, } from "metabase/meta/Parameter"; import * as Query from "metabase/lib/query/query"; @@ -190,20 +191,17 @@ export function applyParameters( parameter_id: parameter.id, }, ); + if (mapping) { // mapped target, e.x. on a dashboard - datasetQuery.parameters.push({ - type: parameter.type, - target: mapping.target, - value: value, - }); + datasetQuery.parameters.push( + mapUIParameterToQueryParameter(parameter.type, value, mapping.target), + ); } else if (parameter.target) { // inline target, e.x. on a card - datasetQuery.parameters.push({ - type: parameter.type, - target: parameter.target, - value: value, - }); + datasetQuery.parameters.push( + mapUIParameterToQueryParameter(parameter.type, value, parameter.target), + ); } } diff --git a/frontend/src/metabase/meta/Dashboard.js b/frontend/src/metabase/meta/Dashboard.js index 1c177cd8bc623..7b2f0c0e6752d 100644 --- a/frontend/src/metabase/meta/Dashboard.js +++ b/frontend/src/metabase/meta/Dashboard.js @@ -1,4 +1,5 @@ import Question from "metabase-lib/lib/Question"; +import { ExpressionDimension } from "metabase-lib/lib/Dimension"; import type Metadata from "metabase-lib/lib/metadata/Metadata"; import type { Card } from "metabase-types/types/Card"; @@ -45,6 +46,12 @@ export const PARAMETER_SECTIONS: ParameterSection[] = [ description: t`User ID, product ID, event ID, etc.`, options: [], }, + { + id: "number", + name: t`Number`, + description: t`Subtotal, Age, Price, Quantity, etc.`, + options: [], + }, { id: "category", name: t`Other Categories`, @@ -92,7 +99,10 @@ export function getParameterMappingOptions( name: dimension.displayName(), icon: dimension.icon(), target: ["dimension", dimension.mbql()], - isForeign: !!(dimension.fk() || dimension.joinAlias()), + // these methods don't exist on instances of ExpressionDimension + isForeign: !!(dimension instanceof ExpressionDimension + ? false + : dimension.fk() || dimension.joinAlias()), })), ), ); @@ -133,7 +143,7 @@ export function createParameter( option: ParameterOption, parameters: Parameter[] = [], ): Parameter { - let name = option.name; + let name = option.combinedName || option.name; let nameIndex = 0; // get a unique name while (_.any(parameters, p => p.name === name)) { diff --git a/frontend/src/metabase/meta/Parameter.js b/frontend/src/metabase/meta/Parameter.js index 872780455905c..ccb8dacc956e4 100644 --- a/frontend/src/metabase/meta/Parameter.js +++ b/frontend/src/metabase/meta/Parameter.js @@ -21,8 +21,17 @@ import type Field from "metabase-lib/lib/metadata/Field"; import Dimension, { FieldDimension } from "metabase-lib/lib/Dimension"; import moment from "moment"; import { t } from "ttag"; +import _ from "underscore"; import * as FIELD_REF from "metabase/lib/query/field_ref"; -import { isNumericBaseType } from "metabase/lib/schema_metadata"; +import { + isNumericBaseType, + doesOperatorExist, + getOperatorByTypeAndName, + STRING, + NUMBER, + PRIMARY_KEY, +} from "metabase/lib/schema_metadata"; + import Variable, { TemplateTagVariable } from "metabase-lib/lib/Variable"; type DimensionFilter = (dimension: Dimension) => boolean; @@ -30,6 +39,91 @@ type TemplateTagFilter = (tag: TemplateTag) => boolean; type FieldPredicate = (field: Field) => boolean; type VariableFilter = (variable: Variable) => boolean; +export const PARAMETER_OPERATOR_TYPES = { + number: [ + { + operator: "=", + name: t`Equal to`, + }, + { + operator: "!=", + name: t`Not equal to`, + }, + { + operator: "between", + name: t`Between`, + }, + { + operator: ">=", + name: t`Greater than or equal to`, + }, + { + operator: "<=", + name: t`Less than or equal to`, + }, + // { + // operator: "all-options", + // name: t`All options`, + // description: t`Contains all of the above`, + // }, + ], + string: [ + { + operator: "=", + name: t`Dropdown`, + description: t`Select one or more values from a list or search box.`, + }, + { + operator: "!=", + name: t`Is not`, + description: t`Exclude one or more specific values.`, + }, + { + operator: "contains", + name: t`Contains`, + description: t`Match values that contain the entered text.`, + }, + { + operator: "does-not-contain", + name: t`Does not contain`, + description: t`Filter out values that contain the entered text.`, + }, + { + operator: "starts-with", + name: t`Starts with`, + description: t`Match values that begin with the entered text.`, + }, + { + operator: "ends-with", + name: t`Ends with`, + description: t`Match values that end with the entered text.`, + }, + // { + // operator: "all-options", + // name: t`All options`, + // description: t`Users can pick from any of the above`, + // }, + ], +}; + +const OPTIONS_WITH_OPERATOR_SUBTYPES = [ + { + section: "location", + operatorType: "string", + sectionName: t`Location`, + }, + { + section: "category", + operatorType: "string", + sectionName: t`Category`, + }, + { + section: "number", + operatorType: "number", + sectionName: t`Number`, + }, +]; + export const PARAMETER_OPTIONS: ParameterOption[] = [ { type: "date/month-year", @@ -62,31 +156,27 @@ export const PARAMETER_OPTIONS: ParameterOption[] = [ menuName: t`All Options`, description: t`Contains all of the above`, }, - { - type: "location/city", - name: t`City`, - }, - { - type: "location/state", - name: t`State`, - }, - { - type: "location/zip_code", - name: t`ZIP or Postal Code`, - }, - { - type: "location/country", - name: t`Country`, - }, { type: "id", name: t`ID`, }, - { - type: "category", - name: t`Category`, - }, -]; + ...OPTIONS_WITH_OPERATOR_SUBTYPES.map(option => + buildOperatorSubtypeOptions(option), + ), +].flat(); + +function buildOperatorSubtypeOptions({ section, operatorType, sectionName }) { + return PARAMETER_OPERATOR_TYPES[operatorType].map(option => ({ + ...option, + combinedName: + operatorType === "string" && option.operator === "=" + ? `${sectionName}` + : sectionName === "Number" + ? `${option.name}` + : `${sectionName} ${option.name.toLowerCase()}`, + type: `${section}/${option.operator}`, + })); +} function fieldFilterForParameter(parameter: Parameter) { return fieldFilterForParameterType(parameter.type); @@ -95,7 +185,7 @@ function fieldFilterForParameter(parameter: Parameter) { function fieldFilterForParameterType( parameterType: ParameterType, ): FieldPredicate { - const [type] = parameterType.split("/"); + const [type] = splitType(parameterType); switch (type) { case "date": return (field: Field) => field.isDate(); @@ -103,25 +193,28 @@ function fieldFilterForParameterType( return (field: Field) => field.isID(); case "category": return (field: Field) => field.isCategory(); + case "location": + return (field: Field) => + field.isCity() || + field.isState() || + field.isZipCode() || + field.isCountry(); + case "number": + return (field: Field) => field.isNumber() && !field.isCoordinate(); } - switch (parameterType) { - case "location/city": - return (field: Field) => field.isCity(); - case "location/state": - return (field: Field) => field.isState(); - case "location/zip_code": - return (field: Field) => field.isZipCode(); - case "location/country": - return (field: Field) => field.isCountry(); - } return (field: Field) => false; } export function parameterOptionsForField(field: Field): ParameterOption[] { return PARAMETER_OPTIONS.filter(option => fieldFilterForParameterType(option.type)(field), - ); + ).map(option => { + return { + ...option, + name: option.combinedName || option.name, + }; + }); } export function dimensionFilterForParameter( @@ -145,7 +238,12 @@ export function variableFilterForParameter( } function tagFilterForParameter(parameter: Parameter): TemplateTagFilter { - const [type, subtype] = parameter.type.split("/"); + const [type, subtype] = splitType(parameter); + const operator = getParameterOperatorName(subtype); + if (operator !== "=") { + return (tag: TemplateTag) => false; + } + switch (type) { case "date": return (tag: TemplateTag) => subtype === "single" && tag.type === "date"; @@ -155,6 +253,8 @@ function tagFilterForParameter(parameter: Parameter): TemplateTagFilter { return (tag: TemplateTag) => tag.type === "number" || tag.type === "text"; case "category": return (tag: TemplateTag) => tag.type === "number" || tag.type === "text"; + case "number": + return (tag: TemplateTag) => tag.type === "number"; } return (tag: TemplateTag) => false; } @@ -314,19 +414,25 @@ export function dateParameterValueToMBQL( } export function stringParameterValueToMBQL( - parameterValue: ParameterValueOrArray, + parameter: Parameter, fieldRef: LocalFieldReference | ForeignFieldReference, ): ?FieldFilter { - // $FlowFixMe: thinks we're returning a nested array which concat does not do - return ["=", fieldRef].concat(parameterValue); + const parameterValue: ParameterValueOrArray = parameter.value; + const [, subtype] = splitType(parameter); + const operatorName = getParameterOperatorName(subtype); + + return [operatorName, fieldRef].concat(parameterValue); } export function numberParameterValueToMBQL( - parameterValue: ParameterValue, + parameter: ParameterInstance, fieldRef: LocalFieldReference | ForeignFieldReference, ): ?FieldFilter { - // $FlowFixMe: thinks we're returning a nested array which concat does not do - return ["=", fieldRef].concat( + const parameterValue: ParameterValue = parameter.value; + const [, subtype] = splitType(parameter); + const operatorName = getParameterOperatorName(subtype); + + return [operatorName, fieldRef].concat( [].concat(parameterValue).map(v => parseFloat(v)), ); } @@ -356,19 +462,81 @@ export function parameterToMBQLFilter( const field = metadata.field(fieldId); // if the field is numeric, parse the value as a number if (isNumericBaseType(field)) { - return numberParameterValueToMBQL(parameter.value, fieldRef); + return numberParameterValueToMBQL(parameter, fieldRef); } else { - return stringParameterValueToMBQL(parameter.value, fieldRef); + return stringParameterValueToMBQL(parameter, fieldRef); } } } export function getParameterIconName(parameterType: ?ParameterType) { - if (/^date\//.test(parameterType || "")) { - return "calendar"; - } else if (/^location\//.test(parameterType || "")) { - return "location"; + const [type] = splitType(parameterType); + switch (type) { + case "date": + return "calendar"; + case "location": + return "location"; + case "category": + return "string"; + case "number": + return "number"; + case "id": + default: + return "label"; + } +} + +export function mapUIParameterToQueryParameter(type, value, target) { + const [fieldType, maybeOperatorName] = splitType(type); + const operatorName = getParameterOperatorName(maybeOperatorName); + + if (fieldType === "location" || fieldType === "category") { + return { + type: `string/${operatorName}`, + value: [].concat(value), + target, + }; + } else if (fieldType === "number") { + return { + type, + value: [].concat(value), + target, + }; } else { - return "label"; + return { type, value, target }; } } + +function getParameterOperatorName(maybeOperatorName) { + return doesOperatorExist(maybeOperatorName) ? maybeOperatorName : "="; +} + +export function deriveFieldOperatorFromParameter(parameter) { + const [parameterType, maybeOperatorName] = splitType(parameter); + const operatorType = getParameterOperatorType(parameterType); + const operatorName = getParameterOperatorName(maybeOperatorName); + + return getOperatorByTypeAndName(operatorType, operatorName); +} + +function getParameterOperatorType(parameterType) { + switch (parameterType) { + case "number": + return NUMBER; + case "location": + case "category": + return STRING; + case "id": + // id can technically be a FK but doesn't matter as both use default filter operators + return PRIMARY_KEY; + default: + return undefined; + } +} + +function splitType(parameterOrType) { + const parameterType = _.isString(parameterOrType) + ? parameterOrType + : (parameterOrType || {}).type || ""; + return parameterType.split("/"); +} diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx index 0a9ac1e07db77..390d7a8c8c4af 100644 --- a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx +++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx @@ -22,7 +22,10 @@ import { makeGetMergedParameterFieldValues, } from "metabase/selectors/metadata"; -import { getParameterIconName } from "metabase/meta/Parameter"; +import { + getParameterIconName, + deriveFieldOperatorFromParameter, +} from "metabase/meta/Parameter"; import S from "./ParameterWidget.css"; @@ -138,7 +141,6 @@ export default class ParameterValueWidget extends Component { metadata, parameter, value, - values, setValue, isEditing, placeholder, @@ -191,9 +193,7 @@ export default class ParameterValueWidget extends Component { > {showTypeIcon && }
    - {hasValue - ? WidgetDefinition.format(value, values) - : placeholderText} + {hasValue ? WidgetDefinition.format(value) : placeholderText}
    ); } else { diff --git a/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx b/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx index 716efd35fe0ad..7768fbb9aeffe 100644 --- a/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx +++ b/frontend/src/metabase/parameters/components/widgets/ParameterFieldWidget.jsx @@ -2,6 +2,7 @@ import React, { Component } from "react"; import ReactDOM from "react-dom"; import { t, ngettext, msgid } from "ttag"; +import _ from "underscore"; import FieldValuesWidget from "metabase/components/FieldValuesWidget"; import Popover from "metabase/components/Popover"; @@ -12,6 +13,12 @@ import Field from "metabase-lib/lib/metadata/Field"; import type { Parameter } from "metabase-types/types/Parameter"; import type { DashboardWithCards } from "metabase-types/types/Dashboard"; +import type { FilterOperator } from "metabase-types/types/Metadata"; +import cx from "classnames"; +import { + getFilterArgumentFormatOptions, + isEqualsOperator, +} from "metabase/lib/schema_metadata"; type Props = { value: any, @@ -22,9 +29,11 @@ type Props = { fields: Field[], parentFocusChanged: boolean => void, + operator?: FilterOperator, dashboard?: DashboardWithCards, parameter?: Parameter, parameters?: Parameter[], + placeholder?: string, }; type State = { @@ -91,11 +100,21 @@ export default class ParameterFieldWidget extends Component<*, Props, State> { } render() { - const { setValue, isEditing, fields, parentFocusChanged } = this.props; - const { isFocused } = this.state; - + const { + setValue, + isEditing, + fields, + parentFocusChanged, + operator, + parameter, + parameters, + dashboard, + } = this.props; + const { isFocused, widgetWidth } = this.state; + const { numFields = 1, multi = false, verboseName } = operator || {}; const savedValue = normalizeValue(this.props.value); const unsavedValue = normalizeValue(this.state.value); + const isEqualsOp = isEqualsOperator(operator); const defaultPlaceholder = isFocused ? "" @@ -138,42 +157,63 @@ export default class ParameterFieldWidget extends Component<*, Props, State> { hasArrow={false} onClose={() => focusChanged(false)} > - { - this.setState({ value }); - }} - placeholder={placeholder} - fields={fields} - multi - autoFocus - color="brand" - style={{ - borderWidth: BORDER_WIDTH, - minWidth: this.state.widgetWidth - ? this.state.widgetWidth + BORDER_WIDTH * 2 - : null, - }} - className="border-bottom" - minWidth={400} - maxWidth={400} - /> - {/* border between input and footer comes from border-bottom on FieldValuesWidget */} -
    - +
    + {verboseName && !isEqualsOp && ( +
    {verboseName}...
    + )} + + {_.times(numFields, index => { + const value = multi ? unsavedValue : [unsavedValue[index]]; + const onValueChange = multi + ? newValues => this.setState({ value: newValues }) + : ([value]) => { + const newValues = [...unsavedValue]; + newValues[index] = value; + this.setState({ value: newValues }); + }; + return ( + + ); + })} + {/* border between input and footer comes from border-bottom on FieldValuesWidget */} +
    + +
    ); diff --git a/frontend/src/metabase/query_builder/components/ExpressionPopover.jsx b/frontend/src/metabase/query_builder/components/ExpressionPopover.jsx index f4aab74634e35..8b82bfe30ef4e 100644 --- a/frontend/src/metabase/query_builder/components/ExpressionPopover.jsx +++ b/frontend/src/metabase/query_builder/components/ExpressionPopover.jsx @@ -12,6 +12,7 @@ import "./ExpressionPopover.css"; export default class ExpressionPopover extends React.Component { state = { error: null, + isBlank: true, }; render() { @@ -26,11 +27,11 @@ export default class ExpressionPopover extends React.Component { name, onChangeName, } = this.props; - const { error } = this.state; + const { error, isBlank } = this.state; - // if onChangeName is provided then a name is required - const isValid = !error && (!onChangeName || name); + const buttonEnabled = !error && !isBlank && (!onChangeName || name); + // if onChangeName is provided then a name is required return (
    @@ -56,6 +57,9 @@ export default class ExpressionPopover extends React.Component { onDone(expression); } }} + onBlankChange={newBlank => { + this.setState({ isBlank: newBlank }); + }} /> {onChangeName && ( onChangeName(e.target.value)} onKeyPress={e => { - if (e.key === "Enter" && isValid) { + if (e.key === "Enter" && buttonEnabled) { onDone(expression); } }} @@ -73,7 +77,7 @@ export default class ExpressionPopover extends React.Component {