Merge pull request #6000 from metabase/card-database-ids

Fix bug with card database_ids nog being populated on native queries

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)))

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}]

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)))]))