Skip to content
Snippets Groups Projects
Unverified Commit 5b0b592a authored by Ryan Laurie's avatar Ryan Laurie Committed by GitHub
Browse files

Add Support for Case-insensitive `contains` dashboard filters (#24582)


* Dashboard Card String Filters are now case-insensitive

This PR is a draft because while it solves the problem of string filters being case sensitive, it doesn't necessarily
do it in the best way.

This isn't necessarily
a bug, but it seems that there is no way for the frontend to set the :case-sensitive true/false option anyway.

For the purposes of this initial attempt at a solution, I have modified the endpoint
` POST "/:dashboard-id/dashcard/:dashcard-id/card/:card-id/query"` to automatically include an option map containing
:case-sensitive false.

The machinery to take this option into consideration already exists with the default :sql ->honeysql function in the `
metabase.driver.sql.query-processor` namespace. See the `like-clause` private function in particular.

Since the query processor is a bit opaque to me at present, I was unable to figure out if there is a proper way that
the frontend could send an options map (or key-value pair) all the way through the qp middleware to the query building
stage. I discovered that if you conj a map `{:case-sensitive false}` to the output of the `to-clause` function in
`metabase.driver.common.parameters.operators`, you will get the desired case-insensitive behavior. So, I modified the
to-clause function in this PR to appropriately conj an options map if one exists.

What I'd like to know:

- is there a super-simple way to pass an option in already that I just missed? (eg. I thought perhaps in the `[:field
13 nil]` that 'nil' could be an options map, but I couldn't get that to work for me)

- is there a middleware approach that I should consider?

- any other options to appropriately hanlde this?

* Revert the endpoint.

If the frontend sends an options map on an options key inside the parameter, this endpoint will pass that on, so no
change is needed.

* include parameter options in datasetQuery

Co-authored-by: default avatarAdam James <adam.vermeer2@gmail.com>
parent cc632e81
No related branches found
No related tags found
No related merge requests found
......@@ -2,6 +2,7 @@ import _ from "underscore";
import { updateIn } from "icepick";
import { normalizeParameterValue } from "metabase/parameters/utils/parameter-values";
import { deriveFieldOperatorFromParameter } from "metabase/parameters/utils/operators";
import * as Q_DEPRECATED from "metabase/lib/query"; // legacy
import Utils from "metabase/lib/utils";
......@@ -116,12 +117,16 @@ export function applyParameters(
);
const type = parameter.type;
const options =
deriveFieldOperatorFromParameter(parameter)?.optionsDefaults;
if (mapping) {
// mapped target, e.x. on a dashboard
datasetQuery.parameters.push({
type,
value: normalizeParameterValue(type, value),
target: mapping.target,
options,
id: parameter.id,
});
} else if (parameter.target) {
......@@ -130,6 +135,7 @@ export function applyParameters(
type,
value: normalizeParameterValue(type, value),
target: parameter.target,
options,
id: parameter.id,
});
}
......
......@@ -56,18 +56,21 @@
(s/defn to-clause :- mbql.s/Filter
"Convert an operator style parameter into an mbql clause. Will also do arity checks and throws an ex-info with
`:type qp.error-type/invalid-parameter` if arity is incorrect."
[{param-type :type [a b :as param-value] :value [_ field :as _target] :target :as _param}]
[{param-type :type [a b :as param-value] :value [_ field :as _target] :target options :options :as _param}]
(verify-type-and-arity field param-type param-value)
(let [field' (params/wrap-field-id-if-needed field)]
(condp = (operator-arity param-type)
:binary
[(keyword (name param-type)) field' a b]
(cond-> [(keyword (name param-type)) field' a b]
(boolean options) (conj options))
:unary
[(keyword (name param-type)) field' a]
(cond-> [(keyword (name param-type)) field' a]
(boolean options) (conj options))
:variadic
(into [(keyword (name param-type)) field'] param-value)
(cond-> (into [(keyword (name param-type)) field'] param-value)
(boolean options) (conj options))
(throw (ex-info (format "Unrecognized operator: %s" param-type)
{:param-type param-type
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment