diff --git a/.dir-locals.el b/.dir-locals.el
index 63008bd395e87a835bb05e3b74a4e31cfe4fe17b..305472b75754a24b24143e6b9cc0ffbab7c152f9 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -23,6 +23,8 @@
(macros/case 0)
(match 1)
(mbql.match/match 1)
+ (mt/test-drivers 1)
+ (mt/query 1)
(mbql.match/match-one 1)
(mbql.match/replace 1)
(mbql.match/replace-in 2)
diff --git a/frontend/src/metabase/dashboard/components/ParametersPopover.jsx b/frontend/src/metabase/dashboard/components/ParametersPopover.jsx
index fcfa5945478c28f530ae1f56d61149820914958e..36722735bd896ea12884f6f81b5e5d22b7cd56ee 100644
--- a/frontend/src/metabase/dashboard/components/ParametersPopover.jsx
+++ b/frontend/src/metabase/dashboard/components/ParametersPopover.jsx
@@ -1,6 +1,6 @@
import React, { Component } from "react";
import { t } from "ttag";
-import { PARAMETER_SECTIONS } from "metabase/meta/Dashboard";
+import { getParameterSections } from "metabase/meta/Dashboard";
import Icon from "metabase/components/Icon";
import { getParameterIconName } from "metabase/meta/Parameter";
import styled from "styled-components";
@@ -32,15 +32,17 @@ export default class ParametersPopover extends Component {
this.state = {};
}
+ PARAMETER_SECTIONS = getParameterSections();
+
render() {
const { section } = this.state;
const { onClose, onAddParameter } = this.props;
if (section == null) {
return (
<ParameterOptionsSectionsPane
- sections={PARAMETER_SECTIONS}
+ sections={this.PARAMETER_SECTIONS}
onSelectSection={selectedSection => {
- const parameterSection = _.findWhere(PARAMETER_SECTIONS, {
+ const parameterSection = _.findWhere(this.PARAMETER_SECTIONS, {
id: selectedSection.id,
});
if (parameterSection && parameterSection.options.length === 1) {
@@ -53,7 +55,9 @@ export default class ParametersPopover extends Component {
/>
);
} else {
- const parameterSection = _.findWhere(PARAMETER_SECTIONS, { id: section });
+ const parameterSection = _.findWhere(this.PARAMETER_SECTIONS, {
+ id: section,
+ });
return (
<ParameterOptionsPane
options={parameterSection && parameterSection.options}
diff --git a/frontend/src/metabase/meta/Dashboard.js b/frontend/src/metabase/meta/Dashboard.js
index f999dfdf5d0606f789fe2bd05778938372fce4a6..d58479732e7ee582abf4fb9e2d58e869f59d5fcb 100644
--- a/frontend/src/metabase/meta/Dashboard.js
+++ b/frontend/src/metabase/meta/Dashboard.js
@@ -1,3 +1,4 @@
+import MetabaseSettings from "metabase/lib/settings";
import Question from "metabase-lib/lib/Question";
import { ExpressionDimension } from "metabase-lib/lib/Dimension";
@@ -12,7 +13,7 @@ import type {
import {
dimensionFilterForParameter,
variableFilterForParameter,
- PARAMETER_OPTIONS,
+ getParameterOptions,
PARAMETER_OPERATOR_TYPES,
getOperatorDisplayName,
} from "metabase/meta/Parameter";
@@ -29,67 +30,81 @@ export type ParameterSection = {
options: ParameterOption[],
};
-export const PARAMETER_SECTIONS: ParameterSection[] = [
- {
- id: "date",
- name: t`Time`,
- description: t`Date range, relative date, time of day, etc.`,
- options: PARAMETER_OPERATOR_TYPES["date"].map(option => {
- return {
- ...option,
- sectionId: "date",
- combinedName: getOperatorDisplayName(option, "date", t`Date`),
- };
- }),
- },
- {
- id: "location",
- name: t`Location`,
- description: t`City, State, Country, ZIP code.`,
- options: PARAMETER_OPERATOR_TYPES["string"].map(option => {
- return {
- ...option,
- sectionId: "location",
- combinedName: getOperatorDisplayName(option, "string", t`Location`),
- };
- }),
- },
- {
- id: "id",
- name: t`ID`,
- description: t`User ID, product ID, event ID, etc.`,
- options: [
- {
- ..._.findWhere(PARAMETER_OPTIONS, { type: "id" }),
- sectionId: "id",
- },
- ],
- },
- {
- id: "number",
- name: t`Number`,
- description: t`Subtotal, Age, Price, Quantity, etc.`,
- options: PARAMETER_OPERATOR_TYPES["number"].map(option => {
- return {
- ...option,
- sectionId: "number",
- combinedName: getOperatorDisplayName(option, "number", t`Number`),
- };
- }),
- },
- {
- id: "category",
- name: t`Other Categories`,
- description: t`Category, Type, Model, Rating, etc.`,
- options: PARAMETER_OPERATOR_TYPES["string"].map(option => {
- return {
- ...option,
- sectionId: "category",
- combinedName: getOperatorDisplayName(option, "string", t`Category`),
- };
- }),
- },
-];
+const areFieldFilterOperatorsEnabled = () =>
+ MetabaseSettings.get("field-filter-operators-enabled?");
+
+export function getParameterSections(): ParameterSection[] {
+ const parameterOptions = getParameterOptions();
+ const LOCATION_OPTIONS = areFieldFilterOperatorsEnabled()
+ ? PARAMETER_OPERATOR_TYPES["string"].map(option => {
+ return {
+ ...option,
+ sectionId: "location",
+ combinedName: getOperatorDisplayName(option, "string", t`Location`),
+ };
+ })
+ : parameterOptions.filter(option => option.type.startsWith("location"));
+
+ const CATEGORY_OPTIONS = areFieldFilterOperatorsEnabled()
+ ? PARAMETER_OPERATOR_TYPES["string"].map(option => {
+ return {
+ ...option,
+ sectionId: "category",
+ combinedName: getOperatorDisplayName(option, "string", t`Category`),
+ };
+ })
+ : parameterOptions.filter(option => option.type.startsWith("category"));
+
+ return [
+ {
+ id: "date",
+ name: t`Time`,
+ description: t`Date range, relative date, time of day, etc.`,
+ options: PARAMETER_OPERATOR_TYPES["date"].map(option => {
+ return {
+ ...option,
+ sectionId: "date",
+ combinedName: getOperatorDisplayName(option, "date", t`Date`),
+ };
+ }),
+ },
+ {
+ id: "location",
+ name: t`Location`,
+ description: t`City, State, Country, ZIP code.`,
+ options: LOCATION_OPTIONS,
+ },
+ {
+ id: "id",
+ name: t`ID`,
+ description: t`User ID, product ID, event ID, etc.`,
+ options: [
+ {
+ ..._.findWhere(parameterOptions, { type: "id" }),
+ sectionId: "id",
+ },
+ ],
+ },
+ areFieldFilterOperatorsEnabled() && {
+ id: "number",
+ name: t`Number`,
+ description: t`Subtotal, Age, Price, Quantity, etc.`,
+ options: PARAMETER_OPERATOR_TYPES["number"].map(option => {
+ return {
+ ...option,
+ sectionId: "number",
+ combinedName: getOperatorDisplayName(option, "number", t`Number`),
+ };
+ }),
+ },
+ {
+ id: "category",
+ name: t`Other Categories`,
+ description: t`Category, Type, Model, Rating, etc.`,
+ options: CATEGORY_OPTIONS,
+ },
+ ].filter(Boolean);
+}
export function getParameterMappingOptions(
metadata: Metadata,
diff --git a/frontend/src/metabase/meta/Parameter.js b/frontend/src/metabase/meta/Parameter.js
index f612a61deae954910cd492dd0694ec85093daec8..340135d2fb89577a9d089adda3be47ba7220ffd9 100644
--- a/frontend/src/metabase/meta/Parameter.js
+++ b/frontend/src/metabase/meta/Parameter.js
@@ -1,3 +1,4 @@
+import MetabaseSettings from "metabase/lib/settings";
import type { DatasetQuery } from "metabase-types/types/Card";
import type {
TemplateTag,
@@ -38,6 +39,9 @@ type TemplateTagFilter = (tag: TemplateTag) => boolean;
type FieldPredicate = (field: Field) => boolean;
type VariableFilter = (variable: Variable) => boolean;
+const areFieldFilterOperatorsEnabled = () =>
+ MetabaseSettings.get("field-filter-operators-enabled?");
+
export const PARAMETER_OPERATOR_TYPES = {
number: [
{
@@ -160,15 +164,41 @@ const OPTIONS_WITH_OPERATOR_SUBTYPES = [
},
];
-export const PARAMETER_OPTIONS: ParameterOption[] = [
- {
- type: "id",
- name: t`ID`,
- },
- ...OPTIONS_WITH_OPERATOR_SUBTYPES.map(option =>
- buildOperatorSubtypeOptions(option),
- ),
-].flat();
+export function getParameterOptions(): ParameterOption[] {
+ return [
+ {
+ type: "id",
+ name: t`ID`,
+ },
+ ...(areFieldFilterOperatorsEnabled()
+ ? OPTIONS_WITH_OPERATOR_SUBTYPES.map(option =>
+ buildOperatorSubtypeOptions(option),
+ )
+ : [
+ ...PARAMETER_OPERATOR_TYPES["date"],
+ {
+ type: "category",
+ name: t`Category`,
+ },
+ {
+ 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`,
+ },
+ ]),
+ ].flat();
+}
function buildOperatorSubtypeOptions({ type, typeName }) {
return PARAMETER_OPERATOR_TYPES[type].map(option => ({
@@ -204,6 +234,7 @@ function getParameterSubType(parameter) {
function fieldFilterForParameter(parameter: Parameter): FieldPredicate {
const type = getParameterType(parameter);
+ const subtype = getParameterSubType(parameter);
switch (type) {
case "date":
return (field: Field) => field.isDate();
@@ -212,11 +243,24 @@ function fieldFilterForParameter(parameter: Parameter): FieldPredicate {
case "category":
return (field: Field) => field.isCategory();
case "location":
- return (field: Field) =>
- field.isCity() ||
- field.isState() ||
- field.isZipCode() ||
- field.isCountry();
+ return (field: Field) => {
+ if (areFieldFilterOperatorsEnabled()) {
+ return field.isLocation();
+ } else {
+ switch (subtype) {
+ case "city":
+ return field.isCity();
+ case "state":
+ return field.isState();
+ case "zip_code":
+ return field.isZipCode();
+ case "country":
+ return field.isCountry();
+ default:
+ return false;
+ }
+ }
+ };
case "number":
return (field: Field) => field.isNumber() && !field.isCoordinate();
case "string":
@@ -227,14 +271,14 @@ function fieldFilterForParameter(parameter: Parameter): FieldPredicate {
}
export function parameterOptionsForField(field: Field): ParameterOption[] {
- return PARAMETER_OPTIONS.filter(option =>
- fieldFilterForParameter(option)(field),
- ).map(option => {
- return {
- ...option,
- name: option.combinedName || option.name,
- };
- });
+ return getParameterOptions()
+ .filter(option => fieldFilterForParameter(option)(field))
+ .map(option => {
+ return {
+ ...option,
+ name: option.combinedName || option.name,
+ };
+ });
}
export function dimensionFilterForParameter(
@@ -293,7 +337,11 @@ export function getTemplateTagParameters(tags: TemplateTag[]): Parameter[] {
id: tag.id,
type:
tag["widget-type"] ||
- (tag.type === "date" ? "date/single" : "string/="),
+ (tag.type === "date"
+ ? "date/single"
+ : areFieldFilterOperatorsEnabled()
+ ? "string/="
+ : "category"),
target:
tag.type === "dimension"
? ["dimension", ["template-tag", tag.name]]
@@ -510,6 +558,10 @@ export function getParameterIconName(parameter: ?Parameter) {
}
export function mapUIParameterToQueryParameter(type, value, target) {
+ if (!areFieldFilterOperatorsEnabled()) {
+ return { type, value, target };
+ }
+
const [fieldType, maybeOperatorName] = splitType(type);
const operatorName = getParameterOperatorName(maybeOperatorName);
diff --git a/frontend/test/__runner__/backend.js b/frontend/test/__runner__/backend.js
index 0f37e3462b9720e3ef8c1229da6b37931742266e..cc18b6a899233d246ede84c373d48881ffa2ac88 100644
--- a/frontend/test/__runner__/backend.js
+++ b/frontend/test/__runner__/backend.js
@@ -68,6 +68,7 @@ export const BackendResource = createSharedResource("BackendResource", {
(process.env["MB_EDITION"] === "ee" &&
process.env["ENTERPRISE_TOKEN"]) ||
undefined,
+ "MB_FIELD_FILTER_OPERATORS_ENABLED?": "true",
},
stdio:
process.env["DISABLE_LOGGING"] ||
diff --git a/frontend/test/__support__/cypress.js b/frontend/test/__support__/cypress.js
index fcb974cd30d1ead309c5b19d7dae52fd4e1ed8b3..7d4802739a8d9fd8443d9846137b8cfc41647796 100644
--- a/frontend/test/__support__/cypress.js
+++ b/frontend/test/__support__/cypress.js
@@ -315,3 +315,16 @@ export function getIframeBody(selector = "iframe") {
.should("not.be.null")
.then(cy.wrap);
}
+
+export function mockSessionProperty(propertyOrObject, value) {
+ cy.intercept("GET", "/api/session/properties", req => {
+ req.reply(res => {
+ if (typeof propertyOrObject === "object") {
+ Object.assign(res.body, propertyOrObject);
+ }
+ {
+ res.body[propertyOrObject] = value;
+ }
+ });
+ });
+}
diff --git a/frontend/test/metabase/meta/Parameter.unit.spec.js b/frontend/test/metabase/meta/Parameter.unit.spec.js
index 02871c9445c47d806b39a5b55d3fc4ff72b6a07e..044e514767f7a7cf9002d4ef8f37afa8ddc45964 100644
--- a/frontend/test/metabase/meta/Parameter.unit.spec.js
+++ b/frontend/test/metabase/meta/Parameter.unit.spec.js
@@ -7,6 +7,7 @@ import {
mapUIParameterToQueryParameter,
deriveFieldOperatorFromParameter,
} from "metabase/meta/Parameter";
+import MetabaseSettings from "metabase/lib/settings";
describe("metabase/meta/Parameter", () => {
describe("dateParameterValueToMBQL", () => {
@@ -169,6 +170,7 @@ describe("metabase/meta/Parameter", () => {
isCountry: () => false,
isNumber: () => false,
isString: () => false,
+ isLocation: () => false,
};
it("should return relevantly typed options for date field", () => {
const dateField = {
@@ -183,14 +185,14 @@ describe("metabase/meta/Parameter", () => {
});
it("should return relevantly typed options for location field", () => {
- const stringField = {
+ const locationField = {
...field,
- isString: () => true,
+ isCity: () => true,
};
- const availableOptions = parameterOptionsForField(stringField);
+ const availableOptions = parameterOptionsForField(locationField);
expect(
availableOptions.length > 0 &&
- availableOptions.every(option => option.type.startsWith("string")),
+ availableOptions.every(option => option.type.startsWith("location")),
).toBe(true);
});
});
@@ -214,7 +216,22 @@ describe("metabase/meta/Parameter", () => {
});
});
- describe("mapParameterTypeToFieldType", () => {
+ describe("mapParameterTypeToFieldType (field-filter-operators-enabled? === true)", () => {
+ let fieldFilterOperatorsEnabled;
+ beforeAll(() => {
+ fieldFilterOperatorsEnabled = MetabaseSettings.get(
+ "field-filter-operators-enabled?",
+ );
+ MetabaseSettings.set("field-filter-operators-enabled?", true);
+ });
+
+ afterAll(() => {
+ MetabaseSettings.set(
+ "field-filter-operators-enabled?",
+ fieldFilterOperatorsEnabled,
+ );
+ });
+
it("should return the proper parameter type of location/category parameters", () => {
expect(mapUIParameterToQueryParameter("category", "foo", "bar")).toEqual({
type: "string/=",
@@ -274,6 +291,39 @@ describe("metabase/meta/Parameter", () => {
});
});
+ describe("mapParameterTypeToFieldType (field-filter-operators-enabled? === false)", () => {
+ let fieldFilterOperatorsEnabled;
+ beforeAll(() => {
+ fieldFilterOperatorsEnabled = MetabaseSettings.get(
+ "field-filter-operators-enabled?",
+ );
+ MetabaseSettings.set("field-filter-operators-enabled?", false);
+ });
+
+ afterAll(() => {
+ MetabaseSettings.set(
+ "field-filter-operators-enabled?",
+ fieldFilterOperatorsEnabled,
+ );
+ });
+
+ it("return given args in a map", () => {
+ expect(mapUIParameterToQueryParameter("category", "foo", "bar")).toEqual({
+ type: "category",
+ value: "foo",
+ target: "bar",
+ });
+
+ expect(
+ mapUIParameterToQueryParameter("location/city", ["foo"], "bar"),
+ ).toEqual({
+ type: "location/city",
+ value: ["foo"],
+ target: "bar",
+ });
+ });
+ });
+
describe("deriveFieldOperatorFromParameter", () => {
describe("when parameter is associated with an operator", () => {
it("should return relevant operator object", () => {
diff --git a/frontend/test/metabase/scenarios/dashboard/dashboard_data_permissions.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/dashboard_data_permissions.cy.spec.js
index c72529869f9ac4017e2dda8c451ac588cc58c2b6..61fd786a8cca96ad8688b9d3f370cf4b0d422e63 100644
--- a/frontend/test/metabase/scenarios/dashboard/dashboard_data_permissions.cy.spec.js
+++ b/frontend/test/metabase/scenarios/dashboard/dashboard_data_permissions.cy.spec.js
@@ -1,4 +1,9 @@
-import { restore, popover, selectDashboardFilter } from "__support__/cypress";
+import {
+ restore,
+ popover,
+ selectDashboardFilter,
+ mockSessionProperty,
+} from "__support__/cypress";
function filterDashboard(suggests = true) {
cy.visit("/dashboard/1");
@@ -21,6 +26,8 @@ function filterDashboard(suggests = true) {
describe("support > permissions (metabase#8472)", () => {
beforeEach(() => {
+ mockSessionProperty("field-filter-operators-enabled?", true);
+
restore();
cy.signInAsAdmin();
diff --git a/frontend/test/metabase/scenarios/dashboard/old-parameters.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/old-parameters.cy.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..bae6b1a52caef5e578bbae19d5aa6caac1e13ffc
--- /dev/null
+++ b/frontend/test/metabase/scenarios/dashboard/old-parameters.cy.spec.js
@@ -0,0 +1,86 @@
+import { popover, restore, mockSessionProperty } from "__support__/cypress";
+// NOTE: some overlap with parameters-embedded.cy.spec.js
+
+describe("scenarios > dashboard > parameters", () => {
+ beforeEach(() => {
+ mockSessionProperty("field-filter-operators-enabled?", false);
+ restore();
+ cy.signInAsAdmin();
+ });
+
+ it("should hide field operator parameters/show old options", () => {
+ cy.visit("/dashboard/1");
+
+ // Add a filter
+ cy.icon("pencil").click();
+ cy.icon("filter").click();
+
+ cy.findByText("Number").should("not.exist");
+
+ cy.findByText("Location").click();
+ cy.findByText("Contains").should("not.exist");
+ cy.findByText("City").should("exist");
+
+ cy.icon("filter").click();
+ cy.findByText("Contains").should("not.exist");
+ });
+
+ it("should filter by city", () => {
+ cy.visit("/dashboard/1");
+
+ cy.icon("pencil").click();
+ cy.icon("filter").click();
+
+ cy.findByText("Location").click();
+ cy.findByText("City").click();
+
+ cy.findByText("Select…").click();
+ popover().within(() => {
+ cy.findByText("City").click();
+ });
+
+ cy.findByText("No default").click();
+ cy.findByPlaceholderText("Search by City")
+ .click()
+ .type("B");
+ cy.findByText("Baker").click();
+ cy.findByText("Add filter").click();
+ cy.get(".Button--primary")
+ .contains("Done")
+ .click();
+
+ cy.findByText("Save").click();
+ cy.findByText("You're editing this dashboard.").should("not.exist");
+ cy.findByText("Baker");
+
+ cy.contains("of 8");
+ });
+
+ it("should filter by category", () => {
+ cy.visit("/dashboard/1");
+
+ cy.icon("pencil").click();
+ cy.icon("filter").click();
+
+ cy.findByText("Other Categories").click();
+
+ cy.findByText("Select…").click();
+ popover().within(() => {
+ cy.findByText("Name").click();
+ });
+
+ cy.contains("Done").click();
+
+ cy.findByText("Save").click();
+ cy.findByText("You're editing this dashboard.").should("not.exist");
+
+ cy.findByText("Category").click();
+ cy.findByPlaceholderText("Search by Name")
+ .click()
+ .type("bb");
+ cy.findByText("Abbie Parisian").click();
+ cy.findByText("Add filter").click();
+
+ cy.contains("of 18");
+ });
+});
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo/parameters.clj b/modules/drivers/mongo/src/metabase/driver/mongo/parameters.clj
index 12d4db9d1a8fe7403beec6293e74bf0295634f0f..def6e73faece6d01fd36d8e43ad277f7e6e3caac 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo/parameters.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo/parameters.clj
@@ -123,6 +123,7 @@
[:template-tag
[:field (field->name (:field v) false)
{:base-type (get-in v [:field :base_type])}]])
+ params/throw-if-field-filter-operators-not-enabled
ops/to-clause
;; desugar only impacts :does-not-contain -> [:not [:contains ... but it prevents
;; an optimization of [:= 'field 1 2 3] -> [:in 'field [1 2 3]] since that
diff --git a/project.clj b/project.clj
index 279582d8a12a2759c87cf93adb98a28caf1b5ab3..84e4f01a4f89179cd1ec61d1330da8a8b0d9ac70 100644
--- a/project.clj
+++ b/project.clj
@@ -237,6 +237,7 @@
:env
{:mb-run-mode "dev"
+ :mb-field-filter-operators-enabled? "true"
:mb-test-setting-1 "ABCDEFG"}
:jvm-opts
@@ -330,6 +331,7 @@
{:mb-run-mode "test"
:mb-db-in-memory "true"
:mb-jetty-join "false"
+ :mb-field-filter-operators-enabled? "true"
:mb-api-key "test-api-key"
;; use a random port between 3001 and 3501. That way if you run multiple sets of tests at the same time locally
;; they won't stomp on each other
diff --git a/src/metabase/api/embed.clj b/src/metabase/api/embed.clj
index 3834def9b3d758e239311e5ccb70917859e968f8..8340697927b206f7ec3c7c64f31a48f99587e459 100644
--- a/src/metabase/api/embed.clj
+++ b/src/metabase/api/embed.clj
@@ -23,6 +23,7 @@
[metabase.api.dashboard :as dashboard-api]
[metabase.api.dataset :as dataset-api]
[metabase.api.public :as public-api]
+ [metabase.driver.common.parameters :as params]
[metabase.models.card :refer [Card]]
[metabase.models.dashboard :refer [Dashboard]]
[metabase.models.dashboard-card :refer [DashboardCard]]
@@ -134,7 +135,9 @@
:when (and tag-type
(or widget-type (not= tag-type :dimension)))]
{:id (:id tag)
- :type (or widget-type (if (= tag-type :date) :date/single :string/=))
+ :type (or widget-type (cond (= tag-type :date) :date/single
+ (params/field-filter-operators-enabled?) :string/=
+ :else :category))
:target (if (= tag-type :dimension)
[:dimension [:template-tag (:name tag)]]
[:variable [:template-tag (:name tag)]])
diff --git a/src/metabase/driver/common/parameters.clj b/src/metabase/driver/common/parameters.clj
index 44a64a7b63649dd8bc207604ff69b015b64f8283..e85a086b53b83e245f55f9802a69b59d6d528493 100644
--- a/src/metabase/driver/common/parameters.clj
+++ b/src/metabase/driver/common/parameters.clj
@@ -1,10 +1,31 @@
(ns metabase.driver.common.parameters
"Various record types below are used as a convenience for differentiating the different param types."
- (:require [metabase.util.schema :as su]
+ (:require [metabase.models.setting :refer [defsetting]]
+ [metabase.query-processor.error-type :as qp.error-type]
+ [metabase.util.i18n :as i18n :refer [deferred-tru tru]]
+ [metabase.util.schema :as su]
[potemkin.types :as p.types]
[pretty.core :refer [PrettyPrintable]]
[schema.core :as s]))
+(defsetting field-filter-operators-enabled?
+ (deferred-tru "Enable the new field-filter operators")
+ :type :boolean
+ :visibility :public
+ :setter :none)
+
+(defn throw-if-field-filter-operators-not-enabled
+ "Feature flag check for new field filter operations. Assumed already has been checked that `ops/operator?` is
+ true. Throws if the new field filter operators are not enabled. Intended to be removed when feature flag is no
+ longer necessary; designed so it can be used in a threading context and just raised out."
+ {:added "0.39.0"}
+ [field-filter]
+ (if (field-filter-operators-enabled?)
+ field-filter
+ (throw (ex-info (tru "New field filter operators are not enabled")
+ {:type qp.error-type/invalid-parameter
+ :operator (:type field-filter)}))))
+
;; "FieldFilter" is something that expands to a clause like "some_field BETWEEN 1 AND 10"
;;
;; `field` is a Field Toucan instance
diff --git a/src/metabase/driver/sql/parameters/substitution.clj b/src/metabase/driver/sql/parameters/substitution.clj
index f0987281bb29a7b8a793696f99cb9f3771f1d69a..fd6e7a118046b07bd3dd9190bf89597d1e9ad09a 100644
--- a/src/metabase/driver/sql/parameters/substitution.clj
+++ b/src/metabase/driver/sql/parameters/substitution.clj
@@ -243,9 +243,11 @@
(cond
(ops/operator? param-type)
(let [[snippet & args]
- (->> (ops/to-clause (assoc params :target
- [:template-tag [:field (field->identifier driver field param-type)
- {:base-type (:base_type field)}]]))
+ (->> (assoc params :target
+ [:template-tag [:field (field->identifier driver field param-type)
+ {:base-type (:base_type field)}]])
+ i/throw-if-field-filter-operators-not-enabled
+ ops/to-clause
mbql.u/desugar-filter-clause
wrap-value-literals/wrap-value-literals-in-mbql
(sql.qp/->honeysql driver)
diff --git a/src/metabase/query_processor/middleware/parameters/mbql.clj b/src/metabase/query_processor/middleware/parameters/mbql.clj
index e5e364365d7478e3bc30c60f4648c7f5748d55e4..d9283ae92e69881e2c9a007036fb7595ffa39869 100644
--- a/src/metabase/query_processor/middleware/parameters/mbql.clj
+++ b/src/metabase/query_processor/middleware/parameters/mbql.clj
@@ -1,6 +1,7 @@
(ns metabase.query-processor.middleware.parameters.mbql
"Code for handling parameter substitution in MBQL queries."
- (:require [metabase.driver.common.parameters.dates :as date-params]
+ (:require [metabase.driver.common.parameters :as i]
+ [metabase.driver.common.parameters.dates :as date-params]
[metabase.driver.common.parameters.operators :as ops]
[metabase.mbql.schema :as mbql.s]
[metabase.mbql.util :as mbql.u]
@@ -45,7 +46,7 @@
[{param-type :type, param-value :value, [_ field :as target] :target, :as param}]
(cond
(ops/operator? param-type)
- (ops/to-clause param)
+ (ops/to-clause (i/throw-if-field-filter-operators-not-enabled param))
;; multipe values. Recursively handle them all and glue them all together with an OR clause
(sequential? param-value)
(mbql.u/simplify-compound-filter
diff --git a/test/metabase/driver/sql/parameters/substitute_test.clj b/test/metabase/driver/sql/parameters/substitute_test.clj
index 4175882fff7ade321bf789e697a95f5c554463f3..e3934d1cef4edddd3163df124eb14fa93f111685 100644
--- a/test/metabase/driver/sql/parameters/substitute_test.clj
+++ b/test/metabase/driver/sql/parameters/substitute_test.clj
@@ -166,7 +166,17 @@
(substitute query {"param" (i/map->FieldFilter
{:field (Field (mt/id :venues field))
:value {:type operator
- :value value}})})))))))))
+ :value value}})})))))))
+ (testing "Throws if not enabled (#15488)"
+ (with-redefs [i/field-filter-operators-enabled? (constantly false)]
+ (is (= :invalid-parameter
+ (try
+ (substitute ["select * from venues where " (param "param")]
+ {"param" (i/map->FieldFilter
+ {:field (Field (mt/id :venues :price))
+ :value {:type :number/>=
+ :value [3]}})})
+ (catch Exception e (:type (ex-data e))))))))))
;;; -------------------------------------------- Referenced Card Queries ---------------------------------------------
diff --git a/test/metabase/query_processor/middleware/parameters/mbql_test.clj b/test/metabase/query_processor/middleware/parameters/mbql_test.clj
index af3e53a33ce3c76eabbbb29ab06e5254c5f930f4..ee9f2106d9a57e13d3e91219097a0f4934bfafc7 100644
--- a/test/metabase/query_processor/middleware/parameters/mbql_test.clj
+++ b/test/metabase/query_processor/middleware/parameters/mbql_test.clj
@@ -2,8 +2,10 @@
"Tests for *MBQL* parameter substitution."
(:require [clojure.test :refer :all]
[metabase.driver :as driver]
+ [metabase.driver.common.parameters :as params]
[metabase.mbql.normalize :as normalize]
[metabase.query-processor :as qp]
+ [metabase.query-processor.error-type :as qp.error-type]
[metabase.query-processor.middleware.parameters.mbql :as mbql-params]
[metabase.test :as mt]))
@@ -159,7 +161,18 @@
:parameters [{:name "name"
:type :string/starts-with
:target $name
- :value ["B"]}]})))))))))
+ :value ["B"]}]})))))
+ (with-redefs [params/field-filter-operators-enabled? (constantly false)]
+ (testing "Throws if not enabled (#15488)"
+ (is (= {:type qp.error-type/invalid-parameter
+ :operator :number/between}
+ (try (f (mt/query venues
+ {:query {:aggregation [[:count]]}
+ :parameters [{:name "price"
+ :type :number/between
+ :target $price
+ :value [2 5]}]}))
+ (catch Exception e (ex-data e)))))))))))
(deftest basic-where-test
(mt/test-drivers (params-test-drivers)