Skip to content
Snippets Groups Projects
Unverified Commit 7d84f6a1 authored by Mark Bastian's avatar Mark Bastian Committed by GitHub
Browse files

Fix TemplateTag id type discrepancy (#31257)

There was a discrepancy between TemplateTag id types as defined in metabase.lib.native (uuid) and metabase.mbql.schema (string). This PR makes string the uniform type for both. This results in errors when code using different specs for the same logical entity is used together. Here's an breaking example:

```
(let [card-id 328 ;; This is a valid card locally
      q   (str "SELECT * FROM {{#" card-id "}} LIMIT 2")
      tt  (lib-native/extract-template-tags q)
      res (qp/process-query
            {:database 1
             :type     :native
             :native   {:query         q
                        :template-tags tt}})]
  (is (some? res)))
```
This will break since the tt generated id is a uuid but process-query expects a string id.

Fixes #31252 
parent 92263e68
No related merge requests found
......@@ -18,7 +18,7 @@
(def ^:private TemplateTag
[:map
[:type [:enum :text :snippet :card]]
[:id :uuid]
[:id :string]
[:name ::common/non-blank-string]
[:display-name {:js/prop "display-name" :optional true} ::common/non-blank-string]
[:snippet-name {:js/prop "snippet-name" :optional true} ::common/non-blank-string]
......@@ -60,7 +60,7 @@
(defn- fresh-tag [tag-name]
{:type :text
:name tag-name
:id (m/random-uuid)})
:id (str (m/random-uuid))})
(defn- finish-tag [{tag-name :name :as tag}]
(merge tag
......
......@@ -41,16 +41,16 @@
(is (=? {"snippet:foo" {:type :snippet
:name "snippet:foo"
:snippet-name "foo"
:id uuid?}}
:id string?}}
(lib.native/extract-template-tags "SELECT * FROM table WHERE {{snippet:foo}}")))
(is (=? {"snippet:foo" {:type :snippet
:name "snippet:foo"
:snippet-name "foo"
:id uuid?}
:id string?}
"snippet: foo" {:type :snippet
:name "snippet: foo"
:snippet-name "foo"
:id uuid?}}
:id string?}}
;; TODO: This should probably be considered a bug - whitespace matters for the name.
(lib.native/extract-template-tags "SELECT * FROM {{snippet: foo}} WHERE {{snippet:foo}}"))))
......@@ -58,7 +58,7 @@
(let [old-tag {:type :text
:name "foo"
:display-name "Foo"
:id (m/random-uuid)}]
:id (str (m/random-uuid))}]
(testing "changes display-name if the original is not customized"
(is (=? {"bar" {:type :text
:name "bar"
......@@ -78,7 +78,7 @@
(let [other {:type :text
:name "other"
:display-name "Some Var"
:id (m/random-uuid)}]
:id (str (m/random-uuid))}]
(is (=? {"other" other
"bar" {:type :text
:name "bar"
......@@ -92,7 +92,7 @@
(let [mktag (fn [base]
(merge {:type :text
:display-name (u.humanization/name->human-readable-name :simple (:name base))
:id uuid?}
:id string?}
base))
v1 (mktag {:name "foo"})
v2 (mktag {:name "bar"})
......@@ -121,22 +121,22 @@
"#321" c2}
(lib.native/extract-template-tags
"SELECT * FROM {{#321}} WHERE {{baz}} AND {{bar}} AND {{snippet:another snippet}}"
{"foo" (assoc v1 :id (random-uuid))
"#123-card-1" (assoc c1 :id (random-uuid))
"snippet:first snippet" (assoc s1 :id (random-uuid))}))))))
{"foo" (assoc v1 :id (str (random-uuid)))
"#123-card-1" (assoc c1 :id (str (random-uuid)))
"snippet:first snippet" (assoc s1 :id (str (random-uuid)))}))))))
#?(:cljs
(deftest converters-test
(let [clj-tags {"a" {:id #uuid "c5ad010c-632a-4498-b667-9188fbe965f9"
(deftest ^:parallel converters-test
(let [clj-tags {"a" {:id "c5ad010c-632a-4498-b667-9188fbe965f9"
:name "a"
:display-name "A"
:type :text}
"#123-foo" {:id #uuid "7e58e086-5d63-4986-8fe7-87e05dfa4089"
"#123-foo" {:id "7e58e086-5d63-4986-8fe7-87e05dfa4089"
:name "#123-foo"
:display-name "#123-foo"
:type :card
:card-id 123}
"snippet:b" {:id #uuid "604131d0-a74c-4822-b113-8e9515b1a985"
"snippet:b" {:id "604131d0-a74c-4822-b113-8e9515b1a985"
:name "snippet:b"
:display-name "Snippet B"
:type :snippet
......@@ -183,7 +183,7 @@
(is (=? ["select * from venues where id = {{myid}}"
{"myid" {:type :text,
:name "myid",
:id uuid?
:id string?
:display-name "Myid"}}]
((juxt lib/raw-native-query lib/template-tags) query)))
(is (=? ["select * from venues where id = {{myid}} and x = {{y}}"
......
......@@ -7,6 +7,7 @@
[java-time :as t]
[medley.core :as m]
[metabase.driver :as driver]
[metabase.lib.native :as lib-native]
[metabase.models :refer [Card]]
[metabase.query-processor :as qp]
[metabase.test :as mt]
......@@ -73,6 +74,19 @@
(is (= 1
(count-with-params :users :last_login :date/single "2014-08-02T09:30Z" options))))))))))
(deftest template-tag-generation-test
(testing "Generating template tags produces correct types for running process-query (#31252)"
(t2.with-temp/with-temp
[Card {card-id :id} {:dataset true
:dataset_query (mt/native-query {:query "select * from checkins"})}]
(let [q (str "SELECT * FROM {{#" card-id "}} LIMIT 2")
tt (lib-native/extract-template-tags q)
res (qp/process-query
{:database (mt/id)
:type :native
:native {:query q
:template-tags tt}})]
(is (some? res))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Field Filter Params |
......
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