From 47a8d6cd1146430dd5c5f625dc3876a834b928ae Mon Sep 17 00:00:00 2001
From: Ryan Senior <ryan@metabase.com>
Date: Mon, 18 Sep 2017 16:01:27 -0500
Subject: [PATCH] Fix bug with card database_ids not being populated on native
queries
The pre-update function was returning nil for their values which was
then getting stored. This commit does not switch
REPORT_CARD.DATABASE_ID to NOT NULL, that's written up as #5999.
Fixes #5998
---
src/metabase/db/migrations.clj | 9 +++++++++
src/metabase/models/card.clj | 24 ++++++++++++++----------
test/metabase/models/card_test.clj | 17 +++++++++++++++++
3 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/src/metabase/db/migrations.clj b/src/metabase/db/migrations.clj
index d89a9a23396..5fa638637bc 100644
--- a/src/metabase/db/migrations.clj
+++ b/src/metabase/db/migrations.clj
@@ -325,3 +325,12 @@
(when (and stored-site-url
(not= stored-site-url defaulted-site-url))
(setting/set! "site-url" stored-site-url))))
+
+;; There was a bug (#5998) preventing database_id from being persisted with
+;; native query type cards. This migration populates all of the Cards
+;; missing those database ids
+(defmigration ^{:author "senior", :added "0.26.1"} populate-card-database-id
+ (doseq [[db-id cards] (group-by #(get-in % [:dataset_query :database])
+ (db/select [Card :dataset_query :id] :database_id [:= nil]))]
+ (db/update-where! Card {:id [:in (map :id cards)]}
+ :database_id db-id)))
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index a4fce5ed578..32a28402ceb 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -141,7 +141,9 @@
;;; -------------------------------------------------- Lifecycle --------------------------------------------------
-
+(defn- native-query? [query-type]
+ (or (= query-type "native")
+ (= query-type :native)))
(defn- query->database-and-table-ids
"Return a map with `:database-id` and source `:table-id` that should be saved for a Card. Handles queries that use
@@ -149,19 +151,21 @@
normal queries."
[outer-query]
(let [database-id (qputil/get-normalized outer-query :database)
+ query-type (qputil/get-normalized outer-query :type)
source-table (qputil/get-in-normalized outer-query [:query :source-table])]
(cond
- (integer? source-table) {:database-id database-id, :table-id source-table}
- (string? source-table) (let [[_ card-id] (re-find #"^card__(\d+)$" source-table)]
- (db/select-one [Card [:table_id :table-id] [:database_id :database-id]]
- :id (Integer/parseInt card-id))))))
+ (native-query? query-type) {:database-id database-id, :table-id nil}
+ (integer? source-table) {:database-id database-id, :table-id source-table}
+ (string? source-table) (let [[_ card-id] (re-find #"^card__(\d+)$" source-table)]
+ (db/select-one [Card [:table_id :table-id] [:database_id :database-id]]
+ :id (Integer/parseInt card-id))))))
(defn- populate-query-fields [{{query-type :type, :as outer-query} :dataset_query, :as card}]
- (merge (when query-type
- (let [{:keys [database-id table-id]} (query->database-and-table-ids outer-query)]
- {:database_id database-id
- :table_id table-id
- :query_type (keyword query-type)}))
+ (merge (when-let [{:keys [database-id table-id]} (and query-type
+ (query->database-and-table-ids outer-query))]
+ {:database_id database-id
+ :table_id table-id
+ :query_type (keyword query-type)})
card))
(defn- pre-insert [{:keys [dataset_query], :as card}]
diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj
index 40e9c4ecdab..37769bdceaf 100644
--- a/test/metabase/models/card_test.clj
+++ b/test/metabase/models/card_test.clj
@@ -186,3 +186,20 @@
(tu/with-temporary-setting-values [enable-public-sharing false]
(tt/with-temp Card [card {:public_uuid (str (java.util.UUID/randomUUID))}]
(:public_uuid card))))
+
+(defn- dummy-dataset-query [database-id]
+ {:database (data/id)
+ :type :native
+ :native {:query "SELECT count(*) FROM toucan_sightings;"}})
+
+(expect
+ [{:name "some name" :database_id (data/id)}
+ {:name "another name" :database_id (data/id)}]
+ (tt/with-temp Card [{:keys [id] :as card} {:name "some name"
+ :dataset_query (dummy-dataset-query (data/id))
+ :database_id (data/id)}]
+ [(into {} (db/select-one [Card :name :database_id] :id id))
+ (do
+ (db/update! Card id {:name "another name"
+ :dataset_query (dummy-dataset-query (data/id))})
+ (into {} (db/select-one [Card :name :database_id] :id id)))]))
--
GitLab