diff --git a/frontend/src/metabase/dashboard/selectors.unit.spec.js b/frontend/src/metabase/dashboard/selectors.unit.spec.js index 5dd79f1ab898ed57fe617e386de5392c7d0f92b7..0e57a82ba369552e20149b5782b059a1038d3217 100644 --- a/frontend/src/metabase/dashboard/selectors.unit.spec.js +++ b/frontend/src/metabase/dashboard/selectors.unit.spec.js @@ -95,7 +95,7 @@ describe("dashboard/selectors", () => { { id: 1, type: "string/=", - hasOnlyFieldTargets: false, + hasVariableTemplateTagTarget: true, fields: [], }, ]); @@ -118,7 +118,7 @@ describe("dashboard/selectors", () => { { id: 1, type: "string/=", - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, fields: [expect.any(Field)], }, ]); @@ -147,7 +147,7 @@ describe("dashboard/selectors", () => { id: 1, type: "string/=", fields: [expect.any(Field)], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, }, ]); }); @@ -175,7 +175,7 @@ describe("dashboard/selectors", () => { id: 1, type: "string/=", fields: [expect.any(Field)], - hasOnlyFieldTargets: false, + hasVariableTemplateTagTarget: true, }, ]); }); @@ -203,7 +203,7 @@ describe("dashboard/selectors", () => { id: 1, type: "string/=", fields: [expect.any(Field), expect.any(Field)], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, }, ]); }); diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx index 4ebd744bd3240f415286433c99690c44bfc911fd..b84aa35f0d8645126d37ce6acc68a38ec50046a2 100644 --- a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx +++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx @@ -6,12 +6,12 @@ import _ from "underscore"; import { getParameterIconName } from "metabase/parameters/utils/ui"; import { isDashboardParameterWithoutMapping } from "metabase/parameters/utils/dashboards"; -import { isOnlyMappedToFields } from "metabase/parameters/utils/fields"; import { isDateParameter, isNumberParameter, } from "metabase/parameters/utils/parameter-type"; import { getNumberParameterArity } from "metabase/parameters/utils/operators"; + import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; import Icon from "metabase/components/Icon"; import DateSingleWidget from "metabase/components/DateSingleWidget"; @@ -236,35 +236,43 @@ function Widget({ ); } + const normalizedValue = Array.isArray(value) + ? value + : [value].filter(v => v != null); + if (isDateParameter(parameter)) { const DateWidget = DATE_WIDGETS[parameter.type]; return ( <DateWidget value={value} setValue={setValue} onClose={onPopoverClose} /> ); - } else if (isOnlyMappedToFields(parameter)) { - const normalizedValue = Array.isArray(value) - ? value - : [value].filter(v => v != null); - - if (isNumberParameter(parameter)) { - const arity = getNumberParameterArity(parameter); - return ( - <NumberInputWidget - value={normalizedValue} - setValue={value => { - setValue(value); - onPopoverClose(); - }} - arity={arity} - infixText={ - typeof arity === "number" && arity > 1 ? t`and` : undefined - } - autoFocus - placeholder={isEditing ? t`Enter a default value…` : undefined} - /> - ); - } - + } else if (parameter.hasVariableTemplateTagTarget) { + return ( + <TextWidget + value={value} + setValue={setValue} + className={className} + isEditing={isEditing} + commitImmediately={commitImmediately} + placeholder={placeholder} + focusChanged={onFocusChanged} + /> + ); + } else if (isNumberParameter(parameter)) { + const arity = getNumberParameterArity(parameter); + return ( + <NumberInputWidget + value={normalizedValue} + setValue={value => { + setValue(value); + onPopoverClose(); + }} + arity={arity} + infixText={typeof arity === "number" && arity > 1 ? t`and` : undefined} + autoFocus + placeholder={isEditing ? t`Enter a default value…` : undefined} + /> + ); + } else if (!_.isEmpty(parameter.fields)) { return ( <ParameterFieldWidget target={target} @@ -306,10 +314,11 @@ Widget.propTypes = { function getWidgetDefinition(parameter) { if (DATE_WIDGETS[parameter.type]) { return DATE_WIDGETS[parameter.type]; - } else if (isOnlyMappedToFields(parameter)) { - if (isNumberParameter(parameter)) { - return NumberInputWidget; - } + } else if (parameter.hasVariableTemplateTagTarget) { + return TextWidget; + } else if (isNumberParameter(parameter)) { + return NumberInputWidget; + } else if (!_.isEmpty(parameter.fields)) { return ParameterFieldWidget; } else { return TextWidget; diff --git a/frontend/src/metabase/parameters/types.ts b/frontend/src/metabase/parameters/types.ts index c07c62da13cb8117f3ae3e2e53625a1865681c70..5c4fd5c91c704b09ad1952c897fba45fe86d9498 100644 --- a/frontend/src/metabase/parameters/types.ts +++ b/frontend/src/metabase/parameters/types.ts @@ -7,7 +7,7 @@ export interface ValuePopulatedParameter extends Parameter { export interface FieldFilterUiParameter extends ValuePopulatedParameter { fields: Field[]; - hasOnlyFieldTargets?: boolean; + hasVariableTemplateTagTarget?: boolean; } export type UiParameter = FieldFilterUiParameter | ValuePopulatedParameter; diff --git a/frontend/src/metabase/parameters/utils/cards.ts b/frontend/src/metabase/parameters/utils/cards.ts index c9982bf55d0df28e4fc1958eaf8f0b264ce5ffd9..d246bb1caf29855341267626a6c540f04a2046d2 100644 --- a/frontend/src/metabase/parameters/utils/cards.ts +++ b/frontend/src/metabase/parameters/utils/cards.ts @@ -113,11 +113,11 @@ export function getCardUiParameters( return { ...parameter, fields: [field], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, }; } - return { ...parameter }; + return { ...parameter, hasVariableTemplateTagTarget: true }; }); } diff --git a/frontend/src/metabase/parameters/utils/dashboards.ts b/frontend/src/metabase/parameters/utils/dashboards.ts index 2cc17c7fa92786a60ed9481fba136004e246866b..215b3a66648cbf3e4638d4bcaedafbd3ec89a3bb 100644 --- a/frontend/src/metabase/parameters/utils/dashboards.ts +++ b/frontend/src/metabase/parameters/utils/dashboards.ts @@ -2,7 +2,10 @@ import _ from "underscore"; import Question from "metabase-lib/lib/Question"; import { generateParameterId } from "metabase/parameters/utils/parameter-id"; -import { getParameterTargetField } from "metabase/parameters/utils/targets"; +import { + getParameterTargetField, + isVariableTarget, +} from "metabase/parameters/utils/targets"; import { isFieldFilterParameter } from "metabase/parameters/utils/parameter-type"; import { slugify } from "metabase/lib/formatting"; import { @@ -151,8 +154,9 @@ function buildFieldFilterUiParameter( return getTargetField(target, card, metadata); }); const fields = mappedFields.filter((field): field is Field => field != null); - const hasOnlyFieldTargets = - mappedFields.length !== 0 && mappedFields.length === fields.length; + const hasVariableTemplateTagTarget = mappingsForParameter.some(mapping => { + return isVariableTarget(mapping.target); + }); const uniqueFieldsWithFKResolved = _.uniq( fields.map(field => field.target ?? field), field => field.id, @@ -161,7 +165,7 @@ function buildFieldFilterUiParameter( return { ...parameter, fields: uniqueFieldsWithFKResolved, - hasOnlyFieldTargets, + hasVariableTemplateTagTarget, }; } diff --git a/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js b/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js index afb452f537c56f7e67c536217245f0581f08305e..faed2151f33b5dac50342717263a417bc6863d2b 100644 --- a/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js +++ b/frontend/src/metabase/parameters/utils/dashboards.unit.spec.js @@ -799,7 +799,7 @@ describe("metabase/parameters/utils/dashboards", () => { slug: "slug-c", type: "string/=", fields: [], - hasOnlyFieldTargets: false, + hasVariableTemplateTagTarget: false, }, { id: "d", @@ -807,35 +807,35 @@ describe("metabase/parameters/utils/dashboards", () => { type: "number/=", default: [1, 2, 3], fields: [expect.any(Field)], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, }, { id: "e", slug: "slug-e", type: "category", fields: [], - hasOnlyFieldTargets: false, + hasVariableTemplateTagTarget: true, }, { id: "f", slug: "slug-f", type: "string/contains", fields: [expect.any(Field)], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, }, { id: "g", slug: "slug-g", type: "string/starts-with", fields: [expect.any(Field), expect.any(Field)], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, }, { id: "h", slug: "slug-h", type: "string/=", fields: [expect.any(Field)], - hasOnlyFieldTargets: false, + hasVariableTemplateTagTarget: true, }, ]); }); diff --git a/frontend/src/metabase/parameters/utils/fields.js b/frontend/src/metabase/parameters/utils/fields.js index c16fe6852b7a1a588fcf6cf259965d8d0b0f5ca2..52f942bfb5b9b0877fb4276d105685574466b521 100644 --- a/frontend/src/metabase/parameters/utils/fields.js +++ b/frontend/src/metabase/parameters/utils/fields.js @@ -10,7 +10,3 @@ export function hasFields(parameter) { const { fields } = parameter; return Array.isArray(fields) && fields.length > 0; } - -export function isOnlyMappedToFields(parameter) { - return hasFields(parameter) && parameter.hasOnlyFieldTargets; -} diff --git a/frontend/src/metabase/parameters/utils/fields.unit.spec.js b/frontend/src/metabase/parameters/utils/fields.unit.spec.js index 3f00933011fbf2ff750396f3cc28884ec7e0b86b..88d00922f02995e74ad965f70b48dfc6e22e036c 100644 --- a/frontend/src/metabase/parameters/utils/fields.unit.spec.js +++ b/frontend/src/metabase/parameters/utils/fields.unit.spec.js @@ -1,5 +1,5 @@ import Field from "metabase-lib/lib/metadata/Field"; -import { hasFieldValues, hasFields, isOnlyMappedToFields } from "./fields"; +import { hasFieldValues, hasFields } from "./fields"; describe("parameters/utils/fields", () => { describe("hasFieldValues", () => { @@ -38,38 +38,4 @@ describe("parameters/utils/fields", () => { expect(hasFields({ fields: [mockField] })).toBe(true); }); }); - - describe("isOnlyMappedToFields", () => { - it("should be false when the parameter has no fields", () => { - expect( - isOnlyMappedToFields({ fields: [], hasOnlyFieldTargets: false }), - ).toBe(false); - }); - - it("should be false in a broken scenario where it has no fields but claims to only target fields", () => { - expect( - isOnlyMappedToFields({ fields: [], hasOnlyFieldTargets: true }), - ).toBe(false); - }); - - it("should be false when the parameter has fields but is mapped to more than fields", () => { - const mockField = new Field({ id: 1, name: "foo" }); - expect( - isOnlyMappedToFields({ - fields: [mockField], - hasOnlyFieldTargets: false, - }), - ).toBe(false); - }); - - it("should be true when the parameter has fields and is mapped only to fields", () => { - const mockField = new Field({ id: 1, name: "foo" }); - expect( - isOnlyMappedToFields({ - fields: [mockField], - hasOnlyFieldTargets: true, - }), - ).toBe(true); - }); - }); }); diff --git a/frontend/src/metabase/parameters/utils/formatting.ts b/frontend/src/metabase/parameters/utils/formatting.ts index 6e011b186e4c1108f7101b8266978fbddd868e2e..972333a5732805d61b5fcb78f0a45c777d44a41c 100644 --- a/frontend/src/metabase/parameters/utils/formatting.ts +++ b/frontend/src/metabase/parameters/utils/formatting.ts @@ -40,7 +40,7 @@ export function formatParameterValue(value: unknown, parameter: UiParameter) { if (isFieldFilterParameter(parameter)) { // skip formatting field filter parameters mapped to native query variables - if (parameter.hasOnlyFieldTargets === false) { + if (parameter.hasVariableTemplateTagTarget) { return value; } diff --git a/frontend/src/metabase/parameters/utils/formatting.unit.spec.ts b/frontend/src/metabase/parameters/utils/formatting.unit.spec.ts index 9cda301da60f7c19607d124dca577b98150a5f8c..6902b6b3495dc8300026e96d4abdedeb799f0b3e 100644 --- a/frontend/src/metabase/parameters/utils/formatting.unit.spec.ts +++ b/frontend/src/metabase/parameters/utils/formatting.unit.spec.ts @@ -64,7 +64,7 @@ describe("metabase/parameters/utils/formatting", () => { value: 1.111111111111, expected: 1.111111111111, fields: [], - hasOnlyFieldTargets: false, + hasVariableTemplateTagTarget: true, }, { type: "string/=", diff --git a/frontend/src/metabase/parameters/utils/parameter-values.unit.spec.js b/frontend/src/metabase/parameters/utils/parameter-values.unit.spec.js index 573e80521cfd50806554d702eed2c8aabaa6b057..a4b107a8cd256b39435a6146ff5cf5b18c61cd06 100644 --- a/frontend/src/metabase/parameters/utils/parameter-values.unit.spec.js +++ b/frontend/src/metabase/parameters/utils/parameter-values.unit.spec.js @@ -291,7 +291,7 @@ describe("parameters/utils/parameter-values", () => { it("should not normalize date parameters", () => { parameter1.type = "date/foo"; - parameter1.hasOnlyFieldTargets = true; + parameter1.hasVariableTemplateTagTarget = false; expect( getParameterValueFromQueryParams( @@ -306,7 +306,7 @@ describe("parameters/utils/parameter-values", () => { it("should not normalize parameters mapped to non-field targets", () => { parameter1.type = "category"; - parameter1.hasOnlyFieldTargets = false; + parameter1.hasVariableTemplateTagTarget = true; expect( getParameterValueFromQueryParams( @@ -321,7 +321,7 @@ describe("parameters/utils/parameter-values", () => { it("should not normalize empty string parameter values", () => { parameter1.type = "category"; - parameter1.hasOnlyFieldTargets = true; + parameter1.hasVariableTemplateTagTarget = false; expect( getParameterValueFromQueryParams( @@ -336,7 +336,7 @@ describe("parameters/utils/parameter-values", () => { it("should normalize non-date parameters mapped only to field targets", () => { parameter1.type = "category"; - parameter1.hasOnlyFieldTargets = true; + parameter1.hasVariableTemplateTagTarget = false; expect( getParameterValueFromQueryParams( diff --git a/frontend/test/metabase-lib/lib/Question.unit.spec.js b/frontend/test/metabase-lib/lib/Question.unit.spec.js index 3530aa4d13a8cf9aa8344c0eee8a260165288d77..82cbcba35b4ddc8ce5c3db0195a927892420f342 100644 --- a/frontend/test/metabase-lib/lib/Question.unit.spec.js +++ b/frontend/test/metabase-lib/lib/Question.unit.spec.js @@ -1072,7 +1072,7 @@ describe("Question", () => { id: 1, }, ], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, id: "bbb", name: "Foo", slug: "foo", @@ -1081,6 +1081,7 @@ describe("Question", () => { }, { default: undefined, + hasVariableTemplateTagTarget: true, id: "aaa", name: "Bar", slug: "bar", @@ -1118,12 +1119,13 @@ describe("Question", () => { target: ["dimension", ["field", 1, null]], value: "abc", fields: [{ id: 1 }], - hasOnlyFieldTargets: true, + hasVariableTemplateTagTarget: false, }, { type: "category", name: "bar", id: "bar_id", + hasVariableTemplateTagTarget: true, }, ]); });