/api/dash/ tests

.dir-locals.el

@@ -15,4 +15,5 @@
                               (match 1)
                               (macrolet 1)
                               (org-perms-case 1)
+                              (upd 2)
                               (with-credentials 1)))))))

src/metabase/api/common/internal.clj

@@ -125,6 +125,8 @@
 (defn wrap-response-if-needed
   "If RESPONSE isn't already a map with keys `:status` and `:body`, wrap it in one (using status 200)."
   [response]
+  (when (medley.core/boolean? response)                                                            ; Not sure why this is but the JSON serialization middleware
+    (throw (Exception. "Attempted to return a boolean as an API response. This is not allowed!"))) ; barfs if response is just a plain boolean
   (letfn [(is-wrapped? [resp] (and (map? resp)
                                    (contains? resp :status)
                                    (contains? resp :body)))]

src/metabase/api/dash.clj

@@ -7,7 +7,8 @@
             (metabase.models [hydrate :refer [hydrate]]
                              [card :refer [Card]]
                              [dashboard :refer [Dashboard]]
-                             [dashboard-card :refer [DashboardCard]])))
+                             [dashboard-card :refer [DashboardCard]])
+            [metabase.util :as u]))
 
 (defendpoint GET "/" [org f]
   (-> (case (or (keyword f) :all) ; default value for f is `all`
@@ -32,9 +33,11 @@
                    (hydrate :creator :organization [:ordered_cards [:card :creator]] :can_read :can_write))]
     {:dashboard db})) ; why is this returned with this {:dashboard} wrapper?
 
-(defendpoint PUT "/:id" [id :as {{:keys [description name public_perms]} :body}]
+(defendpoint PUT "/:id" [id :as {body :body}]
   (write-check Dashboard id)
-  (upd Dashboard id :description description :name name :public_perms public_perms))
+  (check-500 (->> (u/select-non-nil-keys body :description :name :public_perms)
+                  (medley/mapply upd Dashboard id)))
+  (sel :one Dashboard :id id))
 
 (defendpoint DELETE "/:id" [id]
   (write-check Dashboard id)

src/metabase/models/dashboard.clj

@@ -19,9 +19,10 @@
   (assoc dashboard
          :updated_at (util/new-sql-timestamp)))
 
-(defmethod post-select Dashboard [_ {:keys [id creator_id organization_id] :as dash}]
+(defmethod post-select Dashboard [_ {:keys [id creator_id organization_id description] :as dash}]
   (-> dash
       (assoc :creator (sel-fn :one User :id creator_id)
+             :description (util/jdbc-clob-to-str description)
              :organization (sel-fn :one Org :id organization_id)
              :ordered_cards (sel-fn :many DashboardCard :dashboard_id id))
       assoc-permissions-sets))

src/metabase/util.clj

@@ -104,11 +104,25 @@
   []
   (time/unparse (time/formatters :date-time-no-ms) (coerce/from-long (System/currentTimeMillis))))
 
+(defn buffered-reader->str
+  "JDBC Clobs return `BufferedReaders` when you call `.getCharacterStream`.
+   This Function will read all lines from the reader and return a single unified string."
+  ([^java.io.BufferedReader reader]
+   (->> (buffered-reader->str reader [])       ; get a vector of lines read by the reader
+        (interpose "\n")                       ; add newlines back between them
+        (apply str)))                          ; convert to a single str
+  ([^java.io.BufferedReader reader acc]
+   (if-not (.ready reader) (do (.close reader) ; Read each line from the reader and recurse
+                               acc)            ; until reader is no longer "ready" (i.e. done)
+           (recur reader (conj acc (.readLine reader))))))
 
 (defn jdbc-clob-to-str
   "Convert a `JdbcClob` to a `String` so it can be serialized to JSON."
-  [^org.h2.jdbc.JdbcClob clob]
-  (.getSubString clob 1 (.length clob)))
+  [clob]
+  (when clob
+    (if (string? clob) clob
+        (-> (.getCharacterStream ^org.h2.jdbc.JdbcClob clob)
+            buffered-reader->str))))
 
 
 (defn

test/metabase/api/card_test.clj

@@ -9,7 +9,7 @@
 
 ;; # CARD LIFECYCLE
 
-;; Helper fns
+;; ## Helper fns
 (defn post-card [card-name]
   (-> ((user->client :rasta) :post 200 "card" {:organization (:id @test-org)
                                                :name card-name

test/metabase/api/dash_test.clj

+(ns metabase.api.dash-test
+  "Tests for /api/dash endpoints."
+  (:require [expectations :refer :all]
+            [korma.core :refer :all]
+            [metabase.db :refer :all]
+            (metabase.models [dashboard :refer [Dashboard]]
+                             [user :refer [User]])
+            [metabase.test.util :refer [match-$ expect-eval-actual-first deserialize-dates random-name]]
+            [metabase.test-data :refer :all]))
+
+;; # DASHBOARD LIFECYCLE
+
+;; ## Helper Fns
+(defn create-dash [dash-name]
+  (-> ((user->client :rasta) :post 200 "dash" {:name dash-name
+                                               :organization (:id @test-org)
+                                               :public_perms 0})
+      (deserialize-dates :updated_at :created_at)))
+
+;; ## POST /api/dash
+;; Test that we can create a new Dashboard
+(let [dash-name (random-name)]
+  (expect-eval-actual-first
+      (match-$ (sel :one Dashboard :name dash-name)
+        {:description nil
+         :organization_id 1
+         :name dash-name
+         :creator_id (user->id :rasta)
+         :updated_at $
+         :id $
+         :public_perms 0
+         :created_at $})
+    (create-dash dash-name)))
+
+;; ## GET /api/dash/:id
+;; Test that we can fetch a Dashboard, and that it comes back wrapped in a "dashboard" dictionary (WHY?!)
+(expect-let [dash (create-dash (random-name))]
+  {:dashboard
+   (match-$ dash
+     {:description nil
+      :can_read true
+      :ordered_cards []
+      :creator (-> (sel :one User :id (user->id :rasta))
+                   (select-keys [:email :first_name :last_login :is_superuser :id :last_name :date_joined :common_name]))
+      :can_write true
+      :organization_id (:id @test-org)
+      :name $
+      :organization (-> @test-org
+                        (select-keys [:inherits :logo_url :description :name :slug :id]))
+      :creator_id (user->id :rasta)
+      :updated_at $
+      :id $
+      :public_perms 0
+      :created_at $})}
+  (-> ((user->client :rasta) :get 200 (format "dash/%d" (:id dash)))
+      (deserialize-dates [:dashboard :created_at]
+                         [:dashboard :updated_at]
+                         [:dashboard :creator :date_joined]
+                         [:dashboard :creator :last_login])))
+
+;; ## PUT /api/dash/:id
+;; Test that we can change a Dashboard
+(expect-let [[old-name new-name] (repeatedly 2 random-name)
+             {:keys [id]} (create-dash old-name)
+             get-dash (fn []
+                        (sel :one :fields [Dashboard :name :description :public_perms] :id id))]
+  [{:name old-name
+    :description nil
+    :public_perms 0}
+   {:name new-name
+    :description "My Cool Dashboard"
+    :public_perms 2}
+   {:name old-name
+    :description "My Cool Dashboard"
+    :public_perms 2}]
+  [(get-dash)
+   (do ((user->client :rasta) :put 200 (format "dash/%d" id) {:description "My Cool Dashboard"
+                                                              :public_perms 2
+                                                              :name new-name})
+       (get-dash))
+   ;; See if we can change just the name without affecting other fields
+   (do ((user->client :rasta) :put 200 (format "dash/%d" id) {:name old-name})
+       (get-dash))])
+
+;; ## DELETE /api/dash/:id
+(expect-let [{:keys [id]} (create-dash (random-name))]
+  nil
+  (do ((user->client :rasta) :delete 204 (format "dash/%d" id))
+      (sel :one Dashboard :id id)))
+
+;; # DASHBOARD CARD ENDPOINTS
+
+;; ## POST /api/dash/:id/cards
+
+;; ## DELETE /api/dash/:id/cards

test/metabase/test/util.clj

 (ns metabase.test.util
   "Helper functions and macros for writing unit tests."
   (:require [expectations :refer :all]
-            [medley.core :as medley]
             [metabase.util :as u]))
 
 (declare $->prop)
@@ -9,13 +8,20 @@
 ;; ## Response Deserialization
 
 (defn deserialize-dates
-  "Deserialize date strings with KEYS returned in RESPONSE."
+  "Deserialize date strings with KEYS returned in RESPONSE.
+   Supports plain keywords or vector keysequences like `update-in`.
+
+    (deserialize-dates response :updated_at [:user :last_login])"
   [response & [k & ks]]
   {:pre [(map? response)
-         (keyword? k)]}
-  (let [response (medley/update response k #(some->> (u/parse-iso8601 %)
-                                                     .getTime
-                                                     java.sql.Timestamp.))]
+         (or (keyword? k)
+             (vector? k))]}
+  (let [deserialize-date #(some->> (u/parse-iso8601 %)
+                                   .getTime
+                                   java.sql.Timestamp.)
+        response (cond
+                   (vector? k) (update-in response k deserialize-date)
+                   (keyword? k) (update-in response [k] deserialize-date))]
     (if (empty? ks) response
         (apply deserialize-dates response ks))))
 
@@ -51,7 +57,8 @@
   [source-obj form]
   (or (when (symbol? form)
         (let [[first-char & rest-chars] (name form)]
-          (when (= first-char \$)
+          (when (and (= first-char \$)
+                     (not (empty? rest-chars))) ; don't match just `$`
             (let [kw (->> rest-chars
                           (apply str)
                           keyword)]