Nested Queries + dashboard params fixes

src/metabase/api/database.clj

@@ -109,17 +109,19 @@
      :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))]
+    {:name     "Saved Questions"
+     :id       database/virtual-id
+     :features #{:basic-aggregations}
+     :tables   virtual-tables}))
+
 ;; "Virtual" tables for saved cards simulate the db->schema->table hierarchy by doing fake-db->collection->card
 (defn- add-virtual-tables-for-saved-cards [dbs]
-  (let [virtual-tables (cards-virtual-tables)]
+  (if-let [virtual-db-metadata (saved-cards-virtual-db-metadata)]
     ;; only add the 'Saved Questions' DB if there are Cards that can be used
-    (if-not (seq virtual-tables)
-      dbs
-      (conj (vec dbs)
-            {:name     "Saved Questions"
-             :id       database/virtual-id
-             :features #{:basic-aggregations}
-             :tables   virtual-tables}))))
+    (conj (vec dbs) virtual-db-metadata)
+    dbs))
 
 (defn- dbs-list [include-tables? include-cards?]
   (when-let [dbs (seq (filter mi/can-read? (db/select Database {:order-by [:%lower.name]})))]
@@ -146,6 +148,17 @@
 
 ;;; ------------------------------------------------------------ GET /api/database/:id/metadata ------------------------------------------------------------
 
+;; Since the normal `:id` param in the normal version of the endpoint will never match with negative numbers
+;; we'll create another endpoint to specifically match the ID of the 'virtual' database. The `defendpoint` macro
+;; requires either strings or vectors for the route so we'll have to use a vector and create a regex to only
+;; match the virtual ID (and nothing else).
+(api/defendpoint GET ["/:virtual-db/metadata" :virtual-db (re-pattern (str database/virtual-id))]
+  "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))
+
+
 (defn- db-metadata [id]
   (-> (api/read-check Database id)
       (hydrate [:tables [:fields :target :values] :segments :metrics])

src/metabase/api/field.clj

@@ -64,6 +64,8 @@
     [[:count     (metadata/field-count field)]
      [:distincts (metadata/field-distinct-count field)]]))
 
+(def ^:private empty-field-values
+  {:values {} :human_readable_values {}})
 
 (api/defendpoint GET "/:id/values"
   "If `Field`'s special type derives from `type/Category`, or its base type is `type/Boolean`, return
@@ -71,9 +73,19 @@
   [id]
   (let [field (api/read-check Field id)]
     (if-not (field-should-have-field-values? field)
-      {:values {} :human_readable_values {}}
+      empty-field-values
       (create-field-values-if-needed! field))))
 
+;; match things like GET /field-literal%2Ccreated_at%2Ctype%2FDatetime/values
+;; (this is how things like [field-literal,created_at,type/Datetime] look when URL-encoded)
+(api/defendpoint GET "/field-literal%2C:field-name%2Ctype%2F:field-type/values"
+  "Implementation of the field values endpoint for fields in the Saved Questions 'virtual' DB.
+   This endpoint is just a convenience to simplify the frontend code. It just returns the standard
+   'empty' field values response."
+  ;; we don't actually care what field-name or field-type are, so they're ignored
+  [_ _]
+  empty-field-values)
+
 
 ;; TODO - not sure this is used anymore
 (api/defendpoint POST "/:id/value_map_update"

src/metabase/models/database.clj

@@ -15,6 +15,7 @@
 
 ;;; ------------------------------------------------------------ Constants ------------------------------------------------------------
 
+;; TODO - should this be renamed `saved-cards-virtual-id`?
 (def ^:const ^Integer virtual-id
   "The ID used to signify that a database is 'virtual' rather than physical.
 

test/metabase/api/database_test.clj

@@ -343,17 +343,20 @@
    :features ["basic-aggregations"]
    :tables   card-tables})
 
+(defn- virtual-table-for-card [card & {:as kvs}]
+  (merge {:id           (format "card__%d" (u/get-id card))
+          :db_id        database/virtual-id
+          :display_name (:name card)
+          :schema       "All questions"
+          :description  nil}
+         kvs))
+
 (tt/expect-with-temp [Card [card (card-with-native-query "Kanye West Quote Views Per Month")]]
   (saved-questions-virtual-db
-    {:id           (format "card__%d" (u/get-id card))
-     :db_id        database/virtual-id
-     :display_name "Kanye West Quote Views Per Month"
-     :schema       "All questions"
-     :description  nil})
+    (virtual-table-for-card card))
   (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 database list. The 'Saved Questions' DB should be last on the list
     (last ((user->client :crowberto) :get 200 "database" :include_cards true))))
 
@@ -363,16 +366,8 @@
                       Card       [stamp-card (card-with-native-query "Total Stamp Count", :collection_id (u/get-id stamp-collection))]
                       Card       [coin-card  (card-with-native-query "Total Coin Count",  :collection_id (u/get-id coin-collection))]]
   (saved-questions-virtual-db
-    {:id           (format "card__%d" (u/get-id coin-card))
-     :db_id        database/virtual-id
-     :display_name "Total Coin Count"
-     :schema       "Coins"
-     :description  nil}
-    {:id           (format "card__%d" (u/get-id stamp-card))
-     :db_id        database/virtual-id
-     :display_name "Total Stamp Count"
-     :schema       "Stamps"
-     :description  nil})
+    (virtual-table-for-card coin-card  :schema "Coins")
+    (virtual-table-for-card stamp-card :schema "Stamps"))
   (do
     ;; run the Cards which will populate their result_metadata columns
     (doseq [card [stamp-card coin-card]]
@@ -380,13 +375,6 @@
     ;; Now fetch the database list. The 'Saved Questions' DB should be last on the list. Cards should have their Collection name as their Schema
     (last ((user->client :crowberto) :get 200 "database" :include_cards true))))
 
-(defn- virtual-table-for-card [card]
-  {:id           (format "card__%d" (u/get-id card))
-   :db_id        database/virtual-id
-   :display_name (:name card)
-   :schema       "All questions"
-   :description  nil})
-
 (defn- fetch-virtual-database []
   (some #(when (= (:name %) "Saved Questions")
            %)
@@ -427,3 +415,15 @@
   (saved-questions-virtual-db
     (virtual-table-for-card ok-card))
   (fetch-virtual-database))
+
+
+;; 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))
+  ((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`
+(expect
+  nil
+  ((user->client :crowberto) :get 200 (format "database/%d/metadata" database/virtual-id)))

test/metabase/api/field_test.clj

@@ -9,6 +9,7 @@
              [data :refer :all]
              [util :as tu]]
             [metabase.test.data.users :refer :all]
+            [ring.util.codec :as codec]
             [toucan.db :as db]
             [toucan.util.test :as tt]))
 
@@ -168,6 +169,13 @@
   ((user->client :rasta) :get 200 (format "field/%d/values" (id :venues :id))))
 
 
+;; Check that trying to get values for a 'virtual' field just returns a blank values map
+(expect
+  {:values                {}
+   :human_readable_values {}}
+  ((user->client :rasta) :get 200 (format "field/%s/values" (codec/url-encode "field-literal,created_at,type/Datetime"))))
+
+
 ;; ## POST /api/field/:id/value_map_update
 
 ;; Check that we can set values