Skip to content
Snippets Groups Projects
Unverified Commit 8a529b0e authored by metabase-bot[bot]'s avatar metabase-bot[bot] Committed by GitHub
Browse files

:robot: backported "Fix case-sensitive option leaking to string/=" (#40603)


* Fix case-sensitive option leaking to string/= (#40535)

Fixes #40383

If a native query parameter is set up with a string widget type that
supports a `case-sensitive` option, and that parameter is tied to a
dashboard filter that did not support that option, the option was still
leaking into the filter as an option where options were not expected.

For instance the native query parameter used `string/contains` and the
dashboard used `string/=`, the QP would try to build a filter like `[:=
field value {:case-sensitive false}]` which is an illegal filter.

* Fix moved fn

---------

Co-authored-by: default avatarCase Nelson <case@metabase.com>
Co-authored-by: default avatarChris Truter <crisptrutski@users.noreply.github.com>
parent c8f7b33d
No related branches found
No related tags found
No related merge requests found
......@@ -121,9 +121,20 @@
(let [matching-params (tag-params tag params)
tag-opts (:options tag)
normalize-params (fn [params]
;; remove `:target` which is no longer needed after this point, and add any tag options
(let [params (map #(cond-> (dissoc % :target)
(seq tag-opts) (assoc :options tag-opts))
;; remove `:target` which is no longer needed after this point
;; and add any tag options that are compatible with the new type
(let [params (map (fn [param]
(let [tag-opts (if (and (contains? tag-opts :case-sensitive)
(not (contains? #{:string/contains
:string/does-not-contain
:string/ends-with
:string/starts-with}
(:type param))))
(dissoc tag-opts :case-sensitive)
tag-opts)]
(cond-> (dissoc param :target)
(seq tag-opts)
(assoc :options tag-opts))))
params)]
(if (= (count params) 1)
(first params)
......
......@@ -208,6 +208,44 @@
:target [:dimension [:template-tag "country"]]
:value ["US" "MX"]}]})))))))
(deftest ^:parallel native-with-param-options-different-than-tag-type-test
(testing "Overriding the widget type in parameters should drop case-senstive option when incompatible"
(mt/dataset airports
(is (= {:query "SELECT NAME FROM COUNTRY WHERE (\"PUBLIC\".\"COUNTRY\".\"NAME\" = 'US')"
:params nil}
(qp/compile-and-splice-parameters
{:type :native
:native {:query "SELECT NAME FROM COUNTRY WHERE {{country}}"
:template-tags {"country"
{:name "country"
:display-name "Country"
:type :dimension
:dimension [:field (mt/id :country :name) nil]
:options {:case-sensitive false}
:widget-type :string/contains}}}
:database (mt/id)
:parameters [{:type :string/=
:target [:dimension [:template-tag "country"]]
:value ["US"]}]})))))
(testing "Overriding the widget type in parameters should not drop case-senstive option when compatible"
(mt/dataset airports
(is (= {:query "SELECT NAME FROM COUNTRY WHERE (LOWER(\"PUBLIC\".\"COUNTRY\".\"NAME\") LIKE '%us')"
:params nil}
(qp/compile-and-splice-parameters
{:type :native
:native {:query "SELECT NAME FROM COUNTRY WHERE {{country}}"
:template-tags {"country"
{:name "country"
:display-name "Country"
:type :dimension
:dimension [:field (mt/id :country :name) nil]
:options {:case-sensitive false}
:widget-type :string/contains}}}
:database (mt/id)
:parameters [{:type :string/ends-with
:target [:dimension [:template-tag "country"]]
:value ["US"]}]}))))))
(deftest ^:parallel native-with-spliced-params-test-2
(testing "Make sure we can convert a parameterized query to a native query with spliced params"
(testing "Comma-separated numbers"
......
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