Merge pull request #5431 from metabase/include-fields-metadata-for-virtual-db-metadata

Include fields metadata for Saved Questions 'virtual' DB

src/metabase/api/database.clj

@@ -9,7 +9,9 @@
              [events :as events]
              [sample-data :as sample-data]
              [util :as u]]
-            [metabase.api.common :as api]
+            [metabase.api
+             [common :as api]
+             [table :as table-api]]
             [metabase.models
              [card :refer [Card]]
              [database :as database :refer [Database protected-password]]
@@ -95,22 +97,17 @@
     (filter card-database-supports-nested-queries? <>)
     (remove card-uses-unnestable-aggregation? <>)
     (remove card-has-ambiguous-columns? <>)
-    (map #(dissoc % :result_metadata) <>)      ; frontend has no use for result_metadata just yet so strip it out because it can be big
     (hydrate <> :collection)))
 
 (defn- cards-virtual-tables
   "Return a sequence of 'virtual' Table metadata for eligible Cards.
    (This takes the Cards from `source-query-cards` and returns them in a format suitable for consumption by the Query Builder.)"
-  []
+  [& {:keys [include-fields?]}]
   (for [card (source-query-cards)]
-    {:id           (str "card__" (u/get-id card))
-     :db_id        database/virtual-id
-     :display_name (:name card)
-     :schema       (get-in card [:collection :name] "All questions")
-     :description  (:description card)}))
-
-(defn- saved-cards-virtual-db-metadata []
-  (when-let [virtual-tables (seq (cards-virtual-tables))]
+    (table-api/card->virtual-table card :include-fields? include-fields?)))
+
+(defn- saved-cards-virtual-db-metadata [& {:keys [include-fields?]}]
+  (when-let [virtual-tables (seq (cards-virtual-tables :include-fields? include-fields?))]
     {:name     "Saved Questions"
      :id       database/virtual-id
      :features #{:basic-aggregations}
@@ -156,7 +153,7 @@
   "Endpoint that provides metadata for the Saved Questions 'virtual' database. Used for fooling the frontend
    and allowing it to treat the Saved Questions virtual DB just like any other database."
   []
-  (saved-cards-virtual-db-metadata))
+  (saved-cards-virtual-db-metadata :include-fields? true))
 
 
 (defn- db-metadata [id]

src/metabase/api/table.clj

@@ -98,22 +98,37 @@
                              (partial filter (fn [{:keys [visibility_type]}]
                                                (not= (keyword visibility_type) :sensitive)))))))
 
+(defn- card-result-metadata->virtual-fields
+  "Return a sequence of 'virtual' fields metadata for the 'virtual' table for a Card in the Saved Questions 'virtual' database."
+  [card-id metadata]
+  (for [col metadata]
+    (assoc col
+      :table_id     (str "card__" card-id)
+      :id           [:field-literal (:name col) (or (:base_type col) :type/*)]
+      ;; don't return :special_type if it's a PK or FK because it confuses the frontend since it can't actually be used that way IRL
+      :special_type (when-let [special-type (keyword (:special_type col))]
+                      (when-not (or (isa? special-type :type/PK)
+                                    (isa? special-type :type/FK))
+                        special-type)))))
+
+(defn card->virtual-table
+  "Return metadata for a 'virtual' table for a CARD in the Saved Questions 'virtual' database. Optionally include 'virtual' fields as well."
+  [card & {:keys [include-fields?]}]
+  ;; if collection isn't already hydrated then do so
+  (let [card (hydrate card :colllection)]
+    (cond-> {:id           (str "card__" (u/get-id card))
+             :db_id        database/virtual-id
+             :display_name (:name card)
+             :schema       (get-in card [:collection :name] "All questions")
+             :description  (:description card)}
+      include-fields? (assoc :fields (card-result-metadata->virtual-fields (u/get-id card) (:result_metadata card))))))
+
 (api/defendpoint GET "/card__:id/query_metadata"
   "Return metadata for the 'virtual' table for a Card."
   [id]
-  (let [{metadata :result_metadata, card-name :name} (api/read-check (db/select-one [Card :dataset_query :result_metadata :name], :id id))]
-    {:display_name card-name
-     :db_id        database/virtual-id
-     :id           (str "card__" id)
-     :fields       (for [col metadata]
-                     (assoc col
-                       :table_id     (str "card__" id)
-                       :id           [:field-literal (:name col) (:base_type col)]
-                       ;; don't return :special_type if it's a PK or FK because it confuses the frontend since it can't actually be used that way IRL
-                       :special_type (when-let [special-type (keyword (:special_type col))]
-                                       (when-not (or (isa? special-type :type/PK)
-                                                     (isa? special-type :type/FK))
-                                         special-type))))}))
+  (-> (db/select-one [Card :id :dataset_query :result_metadata :name :description :collection_id], :id id)
+      api/read-check
+      (card->virtual-table :include-fields? true)))
 
 (api/defendpoint GET "/card__:id/fks"
   "Return FK info for the 'virtual' table for a Card. This is always empty, so this endpoint

test/metabase/api/database_test.clj

@@ -420,7 +420,11 @@
 ;; make sure that GET /api/database/:id/metadata works for the Saved Questions 'virtual' database
 (tt/expect-with-temp [Card [card (assoc (card-with-native-query "Birthday Card") :result_metadata [{:name "age_in_bird_years"}])]]
   (saved-questions-virtual-db
-    (virtual-table-for-card card))
+    (assoc (virtual-table-for-card card)
+      :fields [{:name         "age_in_bird_years"
+                :table_id     (str "card__" (u/get-id card))
+                :id           ["field-literal" "age_in_bird_years" "type/*"]
+                :special_type nil}]))
   ((user->client :crowberto) :get 200 (format "database/%d/metadata" database/virtual-id)))
 
 ;; if no eligible Saved Questions exist the virtual DB metadata endpoint should just return `nil`

test/metabase/api/table_test.clj

@@ -467,39 +467,27 @@
                                                   :native   {:query (format "SELECT NAME, ID, PRICE, LATITUDE FROM VENUES")}}}]]
   (let [card-virtual-table-id (str "card__" (u/get-id card))]
     {:display_name "Go Dubs!"
+     :schema       "All questions"
      :db_id        database/virtual-id
      :id           card-virtual-table-id
-     :fields       [{:name         "NAME"
-                     :display_name "Name"
-                     :base_type    "type/Text"
-                     :table_id     card-virtual-table-id
-                     :id           ["field-literal" "NAME" "type/Text"]
-                     :special_type nil}
-                    {:name         "ID"
-                     :display_name "ID"
-                     :base_type    "type/Integer"
-                     :table_id     card-virtual-table-id
-                     :id           ["field-literal" "ID" "type/Integer"]
-                     :special_type nil}
-                    {:name         "PRICE"
-                     :display_name "Price"
-                     :base_type    "type/Integer"
-                     :table_id     card-virtual-table-id
-                     :id           ["field-literal" "PRICE" "type/Integer"]
-                     :special_type nil}
-                    {:name         "LATITUDE"
-                     :display_name "Latitude"
-                     :base_type    "type/Float"
-                     :table_id     card-virtual-table-id
-                     :id           ["field-literal" "LATITUDE" "type/Float"]
-                     :special_type nil}]})
+     :description  nil
+     :fields       (for [[field-name display-name base-type] [["NAME"     "Name"     "type/Text"]
+                                                              ["ID"       "ID"       "type/Integer"]
+                                                              ["PRICE"    "Price"    "type/Integer"]
+                                                              ["LATITUDE" "Latitude" "type/Float"]]]
+                     {:name         field-name
+                      :display_name display-name
+                      :base_type    base-type
+                      :table_id     card-virtual-table-id
+                      :id           ["field-literal" field-name base-type]
+                      :special_type nil})})
   (do
     ;; run the Card which will populate its result_metadata column
     ((user->client :crowberto) :post 200 (format "card/%d/query" (u/get-id card)))
-
     ;; Now fetch the metadata for this "table"
     ((user->client :crowberto) :get 200 (format "table/card__%d/query_metadata" (u/get-id card)))))
 
+
 ;; make sure GET /api/table/:id/fks just returns nothing for 'virtual' tables
 (expect
   []