diff --git a/Dockerfile b/Dockerfile
index 854c3b0eda8a7ad060c42301d51b5711fddedbd8..1793f9eac95f897631a0416e346d5fa833eddaaf 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -16,8 +16,9 @@ ENV LC_CTYPE en_US.UTF-8
# yarn: frontend building
# make: backend building
# gettext: translations
+# java-cacerts: installs updated cacerts to /etc/ssl/certs/java/cacerts
-RUN apk add --update coreutils bash yarn git wget make gettext
+RUN apk add --update coreutils bash yarn git wget make gettext java-cacerts
# lein: backend dependencies and building
ADD https://raw.github.com/technomancy/leiningen/stable/bin/lein /usr/local/bin/lein
@@ -40,9 +41,6 @@ ADD . .
# build the app
RUN bin/build
-# install updated cacerts to /etc/ssl/certs/java/cacerts
-RUN apk add --update java-cacerts
-
# import AWS RDS cert into /etc/ssl/certs/java/cacerts
ADD https://s3.amazonaws.com/rds-downloads/rds-combined-ca-bundle.pem .
RUN keytool -noprompt -import -trustcacerts -alias aws-rds \
diff --git a/bin/docker/Dockerfile b/bin/docker/Dockerfile
index e212a778ec9a6024fb07797a192ed0e221b9e6ac..bdc1cb0a9fe1cabf327f0d0212f1b936e0191752 100644
--- a/bin/docker/Dockerfile
+++ b/bin/docker/Dockerfile
@@ -4,7 +4,7 @@ ENV FC_LANG en-US
ENV LC_CTYPE en_US.UTF-8
# dependencies
-RUN apk add --no-cache bash ttf-dejavu fontconfig
+RUN apk add --update --no-cache bash ttf-dejavu fontconfig
# add Metabase jar
COPY ./metabase.jar /app/
diff --git a/src/metabase/api/pulse.clj b/src/metabase/api/pulse.clj
index 26a4d0d1c82e549af7039d8ca69dec237d899b32..cb353128bd54fb006a3301564aac044903fe55c7 100644
--- a/src/metabase/api/pulse.clj
+++ b/src/metabase/api/pulse.clj
@@ -91,7 +91,6 @@
(let [pulse-before-update (api/write-check Pulse id)]
(check-card-read-permissions cards)
(collection/check-allowed-to-change-collection pulse-before-update pulse-updates)
-
(db/transaction
;; If the collection or position changed with this update, we might need to fixup the old and/or new collection,
;; depending on what changed.
@@ -100,7 +99,7 @@
(pulse/update-pulse!
(assoc (select-keys pulse-updates [:name :cards :channels :skip_if_empty :collection_id :collection_position
:archived])
- :id id))))
+ :id id))))
;; return updated Pulse
(pulse/retrieve-pulse id))
diff --git a/src/metabase/middleware/exceptions.clj b/src/metabase/middleware/exceptions.clj
index 06be9b9031558760681501085b691b3afdb030b6..d36e8522ffb8a1e75b948b1342c4a84eb593940c 100644
--- a/src/metabase/middleware/exceptions.clj
+++ b/src/metabase/middleware/exceptions.clj
@@ -4,7 +4,6 @@
[clojure.string :as str]
[clojure.tools.logging :as log]
[metabase.middleware.security :as mw.security]
- [metabase.util :as u]
[metabase.util.i18n :as ui18n :refer [trs]])
(:import java.sql.SQLException
org.eclipse.jetty.io.EofException))
@@ -56,13 +55,13 @@
status-code
(str message)
- ;; Otherwise it's a 500. Return a body that includes exception & filtered
- ;; stacktrace for debugging purposes
+ ;; Otherwise it's a 500. Return the full Exception for debugging purposes
:else
- (assoc other-info
- :message message
- :type (class e)
- :stacktrace (u/filtered-stacktrace e)))]
+ (merge
+ other-info
+ (Throwable->map e)
+ {:message message
+ :type (class e)}))]
{:status (or status-code 500)
:headers (mw.security/security-headers)
diff --git a/src/metabase/pulse/render/png.clj b/src/metabase/pulse/render/png.clj
index 1d1da38a5b07b0810ff9f35e309c725155dbb8c9..df904d3383105db294e5489e9dde81fe92b779c5 100644
--- a/src/metabase/pulse/render/png.clj
+++ b/src/metabase/pulse/render/png.clj
@@ -12,6 +12,7 @@
[metabase.pulse.render
[common :as common]
[style :as style]]
+ [metabase.util.i18n :refer [trs]]
[schema.core :as s])
(:import cz.vutbr.web.css.MediaSpec
java.awt.Dimension
@@ -24,29 +25,33 @@
org.fit.cssbox.layout.BrowserCanvas
org.w3c.dom.Document))
+(defn- register-font! [filename]
+ (with-open [is (io/input-stream (io/resource filename))]
+ (.registerFont (java.awt.GraphicsEnvironment/getLocalGraphicsEnvironment)
+ (java.awt.Font/createFont java.awt.Font/TRUETYPE_FONT is))))
-(defonce
- ^{:doc "Makes custom fonts available to Java so that CSSBox can render them"
- :private true}
- register-fonts
- (delay (doseq [weight ["regular" "700" "900"]]
- (.registerFont (java.awt.GraphicsEnvironment/getLocalGraphicsEnvironment)
- (java.awt.Font/createFont
- java.awt.Font/TRUETYPE_FONT
- (-> (format "frontend_client/app/fonts/lato-v16-latin/lato-v16-latin-%s.ttf" weight)
- io/resource
- io/input-stream))))))
+(defn- register-fonts! []
+ (try
+ (doseq [weight ["regular" "700" "900"]]
+ (register-font! (format "frontend_client/app/fonts/lato-v16-latin/lato-v16-latin-%s.ttf" weight)))
+ (catch Throwable e
+ (let [message (str (trs "Error registering fonts: Metabase will not be able to send Pulses.")
+ " "
+ (trs "This is a known issue with certain JVMs. See {0} and for more details."
+ "https://github.com/metabase/metabase/issues/7986"))]
+ (log/error e message)
+ (throw (ex-info message {} e))))))
-(when-not *compile-files*
- @register-fonts)
+(defonce ^{:doc "Makes custom fonts available to Java so that CSSBox can render them."
+ :private true
+ :arglists '([])} register-fonts-if-needed!
+ (let [register!* (delay (register-fonts!))]
+ (fn []
+ @register!*)))
(defn- write-image!
[^BufferedImage image, ^String format-name, ^ByteArrayOutputStream output-stream]
- (try
- (ImageIO/write image format-name output-stream)
- (catch javax.imageio.IIOException iioex
- (log/error iioex "Error writing image to output stream")
- (throw iioex))))
+ (ImageIO/write image format-name output-stream))
(defn- dom-analyzer
^DOMAnalyzer [^Document doc, ^StreamDocumentSource doc-source, ^Dimension window-size]
@@ -76,6 +81,7 @@
(defn- render-to-png!
[^String html, ^ByteArrayOutputStream os, width]
+ (register-fonts-if-needed!)
(with-open [is (ByteArrayInputStream. (.getBytes html StandardCharsets/UTF_8))]
(let [doc-source (StreamDocumentSource. is nil "text/html; charset=utf-8")
doc (.parse (DefaultDOMSource. doc-source))
@@ -86,11 +92,15 @@
"Render the Hiccup HTML `content` of a Pulse to a PNG image, returning a byte array."
[{:keys [content]} :- common/RenderedPulseCard
width]
- (let [html (html [:html [:body {:style (style/style
- {:margin 0
- :padding 0
- :background-color :white})}
- content]])]
- (with-open [os (ByteArrayOutputStream.)]
- (render-to-png! html os width)
- (.toByteArray os))))
+ (try
+ (let [html (html [:html [:body {:style (style/style
+ {:margin 0
+ :padding 0
+ :background-color :white})}
+ content]])]
+ (with-open [os (ByteArrayOutputStream.)]
+ (render-to-png! html os width)
+ (.toByteArray os)))
+ (catch Throwable e
+ (log/error e (trs "Error rendering Pulse"))
+ (throw e))))
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 07632c77311fc42d6428946b48549a9777fea47f..eb10cd34e9de93328d23a98d45472acf47303c98 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -25,7 +25,6 @@
[metabase.test
[data :as data]
[util :as tu]]
- [metabase.test.data.users :as test-users]
[toucan.db :as db]
[toucan.util.test :as tt])
(:import java.io.ByteArrayInputStream
@@ -143,7 +142,7 @@
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- card-returned? [model object-or-id card-or-id]
- (contains? (set (for [card ((test-users/user->client :rasta) :get 200 "card", :f model, :model_id (u/get-id object-or-id))]
+ (contains? (set (for [card ((mt/user->client :rasta) :get 200 "card", :f model, :model_id (u/get-id object-or-id))]
(u/get-id card)))
(u/get-id card-or-id)))
@@ -166,7 +165,7 @@
(deftest model-id-requied-when-f-is-database-test
(is (= {:errors {:model_id "model_id is a required parameter when filter mode is 'database'"}}
- ((test-users/user->client :crowberto) :get 400 "card" :f :database))))
+ ((mt/user->client :crowberto) :get 400 "card" :f :database))))
(deftest filter-cards-by-table-test
(testing "Filter cards by table"
@@ -186,7 +185,7 @@
;; Make sure `model_id` is required when `f` is :table
(deftest model_id-requied-when-f-is-table
(is (= {:errors {:model_id "model_id is a required parameter when filter mode is 'table'"}}
- ((test-users/user->client :crowberto) :get 400 "card", :f :table))))
+ ((mt/user->client :crowberto) :get 400 "card", :f :table))))
(deftest filter-by-recent-test
(testing "Filter by `recent`"
@@ -196,21 +195,21 @@
Card [card-4 {:name "Card 4"}]
;; 3 was viewed most recently, followed by 4, then 1. Card 2 was viewed by a different user so
;; shouldn't be returned
- ViewLog [_ {:model "card", :model_id (u/get-id card-1), :user_id (test-users/user->id :rasta)
+ ViewLog [_ {:model "card", :model_id (u/get-id card-1), :user_id (mt/user->id :rasta)
:timestamp #t "2015-12-01"}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-2), :user_id (test-users/user->id :trashbird)
+ ViewLog [_ {:model "card", :model_id (u/get-id card-2), :user_id (mt/user->id :trashbird)
:timestamp #t "2016-01-01"}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (test-users/user->id :rasta)
+ ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (mt/user->id :rasta)
:timestamp #t "2016-02-01"}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-4), :user_id (test-users/user->id :rasta)
+ ViewLog [_ {:model "card", :model_id (u/get-id card-4), :user_id (mt/user->id :rasta)
:timestamp #t "2016-03-01"}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (test-users/user->id :rasta)
+ ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (mt/user->id :rasta)
:timestamp #t "2016-04-01"}]]
(with-cards-in-readable-collection [card-1 card-2 card-3 card-4]
(is (= ["Card 3"
"Card 4"
"Card 1"]
- (map :name ((test-users/user->client :rasta) :get 200 "card", :f :recent)))
+ (map :name ((mt/user->client :rasta) :get 200 "card", :f :recent)))
"Should return cards that were recently viewed by current user only")))))
(deftest filter-by-popular-test
@@ -219,15 +218,15 @@
Card [card-2 {:name "Card 2"}]
Card [card-3 {:name "Card 3"}]
;; 3 entries for card 3, 2 for card 2, none for card 1,
- ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (test-users/user->id :rasta)}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-2), :user_id (test-users/user->id :trashbird)}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-2), :user_id (test-users/user->id :rasta)}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (test-users/user->id :crowberto)}]
- ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (test-users/user->id :rasta)}]]
+ ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (mt/user->id :rasta)}]
+ ViewLog [_ {:model "card", :model_id (u/get-id card-2), :user_id (mt/user->id :trashbird)}]
+ ViewLog [_ {:model "card", :model_id (u/get-id card-2), :user_id (mt/user->id :rasta)}]
+ ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (mt/user->id :crowberto)}]
+ ViewLog [_ {:model "card", :model_id (u/get-id card-3), :user_id (mt/user->id :rasta)}]]
(with-cards-in-readable-collection [card-1 card-2 card-3]
(is (= ["Card 3"
"Card 2"]
- (map :name ((test-users/user->client :rasta) :get 200 "card", :f :popular)))
+ (map :name ((mt/user->client :rasta) :get 200 "card", :f :popular)))
(str "`f=popular` should return cards sorted by number of ViewLog entries for all users; cards with no "
"entries should be excluded"))))))
@@ -238,7 +237,7 @@
Card [card-3 {:name "Card 3", :archived true}]]
(with-cards-in-readable-collection [card-1 card-2 card-3]
(is (= #{"Card 2" "Card 3"}
- (set (map :name ((test-users/user->client :rasta) :get 200 "card", :f :archived))))
+ (set (map :name ((mt/user->client :rasta) :get 200 "card", :f :archived))))
"The set of Card returned with f=archived should be equal to the set of archived cards")))))
(deftest filter-by-fav-test
@@ -246,11 +245,11 @@
(tt/with-temp* [Card [card-1 {:name "Card 1"}]
Card [card-2 {:name "Card 2"}]
Card [card-3 {:name "Card 3"}]
- CardFavorite [_ {:card_id (u/get-id card-1), :owner_id (test-users/user->id :rasta)}]
- CardFavorite [_ {:card_id (u/get-id card-2), :owner_id (test-users/user->id :crowberto)}]]
+ CardFavorite [_ {:card_id (u/get-id card-1), :owner_id (mt/user->id :rasta)}]
+ CardFavorite [_ {:card_id (u/get-id card-2), :owner_id (mt/user->id :crowberto)}]]
(with-cards-in-readable-collection [card-1 card-2 card-3]
(is (= [{:name "Card 1", :favorite true}]
- (for [card ((test-users/user->client :rasta) :get 200 "card", :f :fav)]
+ (for [card ((mt/user->client :rasta) :get 200 "card", :f :fav)]
(select-keys card [:name :favorite]))))))))
@@ -261,34 +260,34 @@
;; Test that we can make a card
(deftest create-a-card
(let [card-name (tu/random-name)]
- (is (= (merge
- card-defaults
- {:name card-name
- :collection_id true
- :collection true
- :creator_id (test-users/user->id :rasta)
- :dataset_query true
- :query_type "query"
- :visualization_settings {:global {:title nil}}
- :database_id true
- :table_id true
- :can_write true
- :dashboard_count 0
- :read_permissions nil
- :result_metadata true
- :creator (merge
- (select-keys (test-users/fetch-user :rasta) [:id :date_joined :last_login])
- {:common_name "Rasta Toucan"
- :is_superuser false
- :is_qbnewb true
- :last_name "Toucan"
- :first_name "Rasta"
- :email "rasta@metabase.com"})})
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [collection]]
- (tu/with-model-cleanup [Card]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (-> ((test-users/user->client :rasta) :post 202 "card"
+ (tu/with-non-admin-groups-no-root-collection-perms
+ (tt/with-temp* [Collection [collection]]
+ (tu/with-model-cleanup [Card]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ (is (= (merge
+ card-defaults
+ {:name card-name
+ :collection_id true
+ :collection true
+ :creator_id (mt/user->id :rasta)
+ :dataset_query true
+ :query_type "query"
+ :visualization_settings {:global {:title nil}}
+ :database_id true
+ :table_id true
+ :can_write true
+ :dashboard_count 0
+ :read_permissions nil
+ :result_metadata true
+ :creator (merge
+ (select-keys (mt/fetch-user :rasta) [:id :date_joined :last_login])
+ {:common_name "Rasta Toucan"
+ :is_superuser false
+ :is_qbnewb true
+ :last_name "Toucan"
+ :first_name "Rasta"
+ :email "rasta@metabase.com"})})
+ (-> ((mt/user->client :rasta) :post 202 "card"
(assoc (card-with-name-and-query card-name (mbql-count-query (data/id) (data/id :venues)))
:collection_id (u/get-id collection)))
(dissoc :created_at :updated_at :id)
@@ -301,33 +300,33 @@
;; Make sure when saving a Card the query metadata is saved (if correct)
(deftest saving-card-saves-query-metadata
- (is (= [{:base_type "type/Integer"
- :display_name "Count Chocula"
- :name "count_chocula"
- :special_type "type/Number"}]
- (tu/with-non-admin-groups-no-root-collection-perms
- (let [metadata [{:base_type :type/Integer
- :display_name "Count Chocula"
- :name "count_chocula"
- :special_type :type/Number}]
- card-name (tu/random-name)]
- (tt/with-temp Collection [collection]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (tu/with-model-cleanup [Card]
- ;; create a card with the metadata
- ((test-users/user->client :rasta) :post 202 "card"
- (assoc (card-with-name-and-query card-name)
- :collection_id (u/get-id collection)
- :result_metadata metadata
- :metadata_checksum (#'results-metadata/metadata-checksum metadata)))
- ;; now check the metadata that was saved in the DB
+ (tu/with-non-admin-groups-no-root-collection-perms
+ (let [metadata [{:base_type :type/Integer
+ :display_name "Count Chocula"
+ :name "count_chocula"
+ :special_type :type/Number}]
+ card-name (tu/random-name)]
+ (tt/with-temp Collection [collection]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ (tu/with-model-cleanup [Card]
+ ;; create a card with the metadata
+ ((mt/user->client :rasta) :post 202 "card"
+ (assoc (card-with-name-and-query card-name)
+ :collection_id (u/get-id collection)
+ :result_metadata metadata
+ :metadata_checksum (#'results-metadata/metadata-checksum metadata)))
+ ;; now check the metadata that was saved in the DB
+ (is (= [{:base_type "type/Integer"
+ :display_name "Count Chocula"
+ :name "count_chocula"
+ :special_type "type/Number"}]
(db/select-one-field :result_metadata Card :name card-name))))))))
;; we should be able to save a Card if the `result_metadata` is *empty* (but not nil) (#9286)
(deftest save-card-with-empty-result-metadata
(is (tu/with-model-cleanup [Card]
;; create a card with the metadata
- ((test-users/user->client :rasta) :post 202 "card"
+ ((mt/user->client :rasta) :post 202 "card"
(assoc (card-with-name-and-query)
:result_metadata []
:metadata_checksum (#'results-metadata/metadata-checksum []))))))
@@ -363,7 +362,7 @@
(tu/throw-if-called qp.async/result-metadata-for-query-async
(tu/with-model-cleanup [Card]
;; create a card with the metadata
- ((test-users/user->client :rasta) :post 202 "card"
+ ((mt/user->client :rasta) :post 202 "card"
(assoc (card-with-name-and-query card-name)
:collection_id (u/get-id collection)
:result_metadata (map fingerprint-integers->doubles metadata)
@@ -390,7 +389,7 @@
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
(tu/with-model-cleanup [Card]
;; create a card with the metadata
- ((test-users/user->client :rasta) :post 202 "card"
+ ((mt/user->client :rasta) :post 202 "card"
(assoc (card-with-name-and-query card-name)
:collection_id (u/get-id collection)
:result_metadata metadata
@@ -418,7 +417,7 @@
(reset! sql-result sql)
(orig driver conn sql params))]
;; create a card with the metadata
- ((test-users/user->client :rasta) :post 202 "card"
+ ((mt/user->client :rasta) :post 202 "card"
(assoc (card-with-name-and-query card-name)
:collection_id (u/get-id collection)
:result_metadata metadata
@@ -436,7 +435,7 @@
(is (= true
(boolean
(when-let [s @sql-result]
- (re-find (re-pattern (str "userID: " (test-users/user->id :rasta)))
+ (re-find (re-pattern (str "userID: " (mt/user->id :rasta)))
s)))))))))))))
;; Make sure we can create a Card with a Collection position
@@ -447,7 +446,7 @@
(let [card-name (tu/random-name)]
(tt/with-temp Collection [collection]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :post 202 "card" (assoc (card-with-name-and-query card-name)
+ ((mt/user->client :rasta) :post 202 "card" (assoc (card-with-name-and-query card-name)
:collection_id (u/get-id collection), :collection_position 1))
(some-> (db/select-one [Card :collection_id :collection_position] :name card-name)
(update :collection_id (partial = (u/get-id collection)))))))))))
@@ -458,7 +457,7 @@
(tu/with-model-cleanup [Card]
(let [card-name (tu/random-name)]
(tt/with-temp Collection [collection]
- ((test-users/user->client :rasta) :post 403 "card" (assoc (card-with-name-and-query card-name)
+ ((mt/user->client :rasta) :post 403 "card" (assoc (card-with-name-and-query card-name)
:collection_id (u/get-id collection)
:collection_position 1))
(some-> (db/select-one [Card :collection_id :collection_position] :name card-name)
@@ -483,9 +482,9 @@
card-defaults
(select-keys card [:id :name :created_at :updated_at])
{:dashboard_count 0
- :creator_id (test-users/user->id :rasta)
+ :creator_id (mt/user->id :rasta)
:creator (merge
- (select-keys (test-users/fetch-user :rasta) [:id :date_joined :last_login])
+ (select-keys (mt/fetch-user :rasta) [:id :date_joined :last_login])
{:common_name "Rasta Toucan"
:is_superuser false
:is_qbnewb true
@@ -502,7 +501,7 @@
:table_id (u/get-id table)
:collection_id (u/get-id collection)
:collection (into {} collection)})
- ((test-users/user->client :rasta) :get 200 (str "card/" (u/get-id card))))))))
+ ((mt/user->client :rasta) :get 200 (str "card/" (u/get-id card))))))))
(deftest check-that-a-user-without-permissions-isn-t-allowed-to-fetch-the-card
(is (= "You don't have permissions to do that."
@@ -513,7 +512,7 @@
;; revoke permissions for default group to this database
(perms/revoke-permissions! (perms-group/all-users) (u/get-id db))
;; now a non-admin user shouldn't be able to fetch this card
- ((test-users/user->client :rasta) :get 403 (str "card/" (u/get-id card))))))))
+ ((mt/user->client :rasta) :get 403 (str "card/" (u/get-id card))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | UPDATING A CARD |
@@ -522,7 +521,7 @@
(deftest updating-a-card-that-doesnt-exist-should-give-a-404
(is (= "Not found."
- ((test-users/user->client :crowberto) :put 404 "card/12345"))))
+ ((mt/user->client :crowberto) :put 404 "card/12345"))))
(deftest test-that-we-can-edit-a-card
@@ -530,7 +529,7 @@
(with-cards-in-writeable-collection card
(is (= "Original Name"
(db/select-one-field :name Card, :id (u/get-id card))))
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:name "Updated Name"})
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:name "Updated Name"})
(is (= "Updated Name"
(db/select-one-field :name Card, :id (u/get-id card)))))))
@@ -539,7 +538,7 @@
(with-cards-in-writeable-collection card
(let [archived? (fn [] (:archived (Card (u/get-id card))))
set-archived! (fn [archived]
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:archived archived})
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:archived archived})
(archived?))]
(is (= false
(archived?)))
@@ -554,119 +553,119 @@
(tt/with-temp* [Collection [collection]
Card [card {:collection_id (u/get-id collection)}]]
(perms/grant-collection-read-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:archived true}))))))
+ ((mt/user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:archived true}))))))
;; Can we clear the description of a Card? (#4738)
(deftest can-we-clear-the-description-of-a-card----4738-
(is (nil? (tt/with-temp Card [card {:description "What a nice Card"}]
(with-cards-in-writeable-collection card
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:description nil})
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:description nil})
(db/select-one-field :description Card :id (u/get-id card)))))))
(deftest description-should-be-blankable-as-well
(is (= ""
(tt/with-temp Card [card {:description "What a nice Card"}]
(with-cards-in-writeable-collection card
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:description ""})
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:description ""})
(db/select-one-field :description Card :id (u/get-id card)))))))
(deftest can-we-update-a-card-s-embedding-params-
(is (= {:abc "enabled"}
(tt/with-temp Card [card]
(tu/with-temporary-setting-values [enable-embedding true]
- ((test-users/user->client :crowberto) :put 202 (str "card/" (u/get-id card)) {:embedding_params {:abc "enabled"}}))
+ ((mt/user->client :crowberto) :put 202 (str "card/" (u/get-id card)) {:embedding_params {:abc "enabled"}}))
(db/select-one-field :embedding_params Card :id (u/get-id card))))))
(deftest we-shouldn-t-be-able-to-update-them-if-we-re-not-an-admin---
(is (= "You don't have permissions to do that."
(tt/with-temp Card [card]
(tu/with-temporary-setting-values [enable-embedding true]
- ((test-users/user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:embedding_params {:abc "enabled"}}))))))
+ ((mt/user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:embedding_params {:abc "enabled"}}))))))
(deftest ---or-if-embedding-isn-t-enabled
(is (= "Embedding is not enabled."
(tt/with-temp Card [card]
(tu/with-temporary-setting-values [enable-embedding false]
- ((test-users/user->client :crowberto) :put 400 (str "card/" (u/get-id card)) {:embedding_params {:abc "enabled"}}))))))
+ ((mt/user->client :crowberto) :put 400 (str "card/" (u/get-id card)) {:embedding_params {:abc "enabled"}}))))))
(deftest make-sure-when-updating-a-card-the-query-metadata-is-saved--if-correct-
- (is (= [{:base_type "type/Integer"
- :display_name "Count Chocula"
- :name "count_chocula"
- :special_type "type/Number"}]
- (let [metadata [{:base_type :type/Integer
- :display_name "Count Chocula"
- :name "count_chocula"
- :special_type :type/Number}]]
- (tt/with-temp Card [card]
- (with-cards-in-writeable-collection card
- ;; update the Card's query
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card))
- {:dataset_query (mbql-count-query)
- :result_metadata metadata
- :metadata_checksum (#'results-metadata/metadata-checksum metadata)})
- ;; now check the metadata that was saved in the DB
+ (let [metadata [{:base_type :type/Integer
+ :display_name "Count Chocula"
+ :name "count_chocula"
+ :special_type :type/Number}]]
+ (tt/with-temp Card [card]
+ (with-cards-in-writeable-collection card
+ ;; update the Card's query
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card))
+ {:dataset_query (mbql-count-query)
+ :result_metadata metadata
+ :metadata_checksum (#'results-metadata/metadata-checksum metadata)})
+ ;; now check the metadata that was saved in the DB
+ (is (= [{:base_type "type/Integer"
+ :display_name "Count Chocula"
+ :name "count_chocula"
+ :special_type "type/Number"}]
(db/select-one-field :result_metadata Card :id (u/get-id card))))))))
(deftest make-sure-when-updating-a-card-the-correct-query-metadata-is-fetched--if-incorrect-
- (is (= [{:base_type "type/BigInteger"
- :display_name "Count"
- :name "count"
- :special_type "type/Quantity"
- :fingerprint {:global {:distinct-count 1
- :nil% 0.0},
- :type {:type/Number {:min 100.0, :max 100.0, :avg 100.0, :q1 100.0, :q3 100.0 :sd nil}}}}]
- (let [metadata [{:base_type :type/BigInteger
- :display_name "Count Chocula"
- :name "count_chocula"
- :special_type :type/Quantity}]]
- (tt/with-temp Card [card]
- (with-cards-in-writeable-collection card
- ;; update the Card's query
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card))
- {:dataset_query (mbql-count-query)
- :result_metadata metadata
- :metadata_checksum "ABC123"}) ; invalid checksum
- ;; now check the metadata that was saved in the DB
+ (let [metadata [{:base_type :type/BigInteger
+ :display_name "Count Chocula"
+ :name "count_chocula"
+ :special_type :type/Quantity}]]
+ (tt/with-temp Card [card]
+ (with-cards-in-writeable-collection card
+ ;; update the Card's query
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card))
+ {:dataset_query (mbql-count-query)
+ :result_metadata metadata
+ :metadata_checksum "ABC123"}) ; invalid checksum
+ ;; now check the metadata that was saved in the DB
+ (is (= [{:base_type "type/BigInteger"
+ :display_name "Count"
+ :name "count"
+ :special_type "type/Quantity"
+ :fingerprint {:global {:distinct-count 1
+ :nil% 0.0},
+ :type {:type/Number {:min 100.0, :max 100.0, :avg 100.0, :q1 100.0, :q3 100.0 :sd nil}}}}]
(db/select-one-field :result_metadata Card :id (u/get-id card))))))))
(deftest can-we-change-the-collection-position-of-a-card-
- (is (= 1
- (tt/with-temp Card [card]
- (with-cards-in-writeable-collection card
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card))
- {:collection_position 1})
+ (tt/with-temp Card [card]
+ (with-cards-in-writeable-collection card
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card))
+ {:collection_position 1})
+ (is (= 1
(db/select-one-field :collection_position Card :id (u/get-id card)))))))
(deftest ---and-unset--unpin--it-as-well-
- (is (nil? (tt/with-temp Card [card {:collection_position 1}]
- (with-cards-in-writeable-collection card
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card))
- {:collection_position nil})
- (db/select-one-field :collection_position Card :id (u/get-id card)))))))
-
-
+ (tt/with-temp Card [card {:collection_position 1}]
+ (with-cards-in-writeable-collection card
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card))
+ {:collection_position nil})
+ (is (= nil
+ (db/select-one-field :collection_position Card :id (u/get-id card)))))))
(deftest ---we-shouldn-t-be-able-to-if-we-don-t-have-permissions-for-the-collection
- (is (nil? (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [collection]
- Card [card {:collection_id (u/get-id collection)}]]
- ((test-users/user->client :rasta) :put 403 (str "card/" (u/get-id card))
- {:collection_position 1})
- (db/select-one-field :collection_position Card :id (u/get-id card)))))))
+ (tu/with-non-admin-groups-no-root-collection-perms
+ (tt/with-temp* [Collection [collection]
+ Card [card {:collection_id (u/get-id collection)}]]
+ ((mt/user->client :rasta) :put 403 (str "card/" (u/get-id card))
+ {:collection_position 1})
+ (is (= nil
+ (db/select-one-field :collection_position Card :id (u/get-id card)))))))
(deftest gets-a-card
- (is (= 1
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [collection]
- Card [card {:collection_id (u/get-id collection), :collection_position 1}]]
- ((test-users/user->client :rasta) :put 403 (str "card/" (u/get-id card))
- {:collection_position nil})
+ (tu/with-non-admin-groups-no-root-collection-perms
+ (tt/with-temp* [Collection [collection]
+ Card [card {:collection_id (u/get-id collection), :collection_position 1}]]
+ ((mt/user->client :rasta) :put 403 (str "card/" (u/get-id card))
+ {:collection_position nil})
+ (is (= 1
(db/select-one-field :collection_position Card :id (u/get-id card)))))))
;;; +----------------------------------------------------------------------------------------------------------------+
-;;; | UPDATING THE POSITION OF A CARDS |
+;;; | Updating the positions of stuff |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- name->position [results]
@@ -677,65 +676,69 @@
"Call the collection endpoint for `collection-id` as `user-kwd`. Will return a map with the names of the items as
keys and their position as the value"
[user-kwd collection-or-collection-id]
- (name->position ((test-users/user->client user-kwd) :get 200 (format "collection/%s/items" (u/get-id collection-or-collection-id)))))
+ (name->position ((mt/user->client user-kwd) :get 200 (format "collection/%s/items" (u/get-id collection-or-collection-id)))))
(defmacro with-ordered-items
"Macro for creating many sequetial collection_position model instances, putting each in `collection`"
[collection model-and-name-syms & body]
`(tt/with-temp* ~(vec (mapcat (fn [idx [model-instance name-sym]]
- [model-instance [name-sym {:name (name name-sym)
- :collection_id `(u/get-id ~collection)
+ [model-instance [name-sym {:name (name name-sym)
+ :collection_id `(u/get-id ~collection)
:collection_position idx}]])
(iterate inc 1)
(partition-all 2 model-and-name-syms)))
- ~@body))
+ (testing (format "\nWith ordered items in Collection %d: %s"
+ (u/get-id ~collection)
+ ~(str/join ", " (for [[model symb] (partition-all 2 model-and-name-syms)]
+ (format "%s %s" (name model) (name symb)))))
+ ~@body)))
(deftest check-to-make-sure-we-can-move-a-card-in-a-collection-of-just-cards
- (is (= {"c" 1
- "a" 2
- "b" 3
- "d" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [collection]
- (with-ordered-items collection [Card a
- Card b
- Card c
- Card d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id c))
- {:collection_position 1})
+ (tu/with-non-admin-groups-no-root-collection-perms
+ (tt/with-temp Collection [collection]
+ (with-ordered-items collection [Card a
+ Card b
+ Card c
+ Card d]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id c))
+ {:collection_position 1})
+ (is (= {"c" 1
+ "a" 2
+ "b" 3
+ "d" 4}
(get-name->collection-position :rasta collection)))))))
(deftest change-the-position-of-the-4th-card-to-1st--all-other-cards-should-inc-their-position
- (is (= {"d" 1
- "a" 2
- "b" 3
- "c" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [collection]
- (with-ordered-items collection [Dashboard a
- Dashboard b
- Pulse c
- Card d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id d))
- {:collection_position 1})
+ (tu/with-non-admin-groups-no-root-collection-perms
+ (tt/with-temp Collection [collection]
+ (with-ordered-items collection [Dashboard a
+ Dashboard b
+ Pulse c
+ Card d]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id d))
+ {:collection_position 1})
+ (is (= {"d" 1
+ "a" 2
+ "b" 3
+ "c" 4}
(get-name->collection-position :rasta collection)))))))
(deftest change-the-position-of-the-1st-card-to-the-4th--all-of-the-other-items-dec
- (is (= {"b" 1
- "c" 2
- "d" 3
- "a" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [collection]
- (with-ordered-items collection [Card a
- Dashboard b
- Pulse c
- Dashboard d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id a))
- {:collection_position 4})
+ (tu/with-non-admin-groups-no-root-collection-perms
+ (tt/with-temp Collection [collection]
+ (with-ordered-items collection [Card a
+ Dashboard b
+ Pulse c
+ Dashboard d]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id a))
+ {:collection_position 4})
+ (is (= {"b" 1
+ "c" 2
+ "d" 3
+ "a" 4}
(get-name->collection-position :rasta collection)))))))
(deftest change-the-position-of-a-card-from-nil-to-2nd--should-adjust-the-existing-items
@@ -751,7 +754,7 @@
Dashboard [_ {:name "c", :collection_id coll-id, :collection_position 2}]
Card [_ {:name "d", :collection_id coll-id, :collection_position 3}]]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id b))
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id b))
{:collection_position 2})
(get-name->collection-position :rasta coll-id))))))
@@ -767,7 +770,7 @@
Dashboard c
Pulse d]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id b))
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id b))
{:collection_position nil})
(get-name->collection-position :rasta collection)))))))
@@ -795,7 +798,7 @@
Dashboard h]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-1)
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-2)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id f))
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id f))
{:collection_id (u/get-id collection-1)})
[(get-name->collection-position :rasta collection-1)
(get-name->collection-position :rasta collection-2)])))))))
@@ -822,7 +825,7 @@
Card h]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-1)
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-2)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id h))
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id h))
{:collection_position 1, :collection_id (u/get-id collection-1)})
[(get-name->collection-position :rasta collection-1)
(get-name->collection-position :rasta collection-2)])))))))
@@ -849,7 +852,7 @@
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
[(get-name->collection-position :rasta collection)
(do
- ((test-users/user->client :rasta) :post 202 "card"
+ ((mt/user->client :rasta) :post 202 "card"
(merge (card-with-name-and-query "a")
{:collection_id (u/get-id collection)
:collection_position 1}))
@@ -876,7 +879,7 @@
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
[(get-name->collection-position :rasta collection)
(do
- ((test-users/user->client :rasta) :post 202 "card"
+ ((mt/user->client :rasta) :post 202 "card"
(merge (card-with-name-and-query "d")
{:collection_id (u/get-id collection)
:collection_position 4}))
@@ -903,7 +906,7 @@
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
[(get-name->collection-position :rasta collection)
(do
- ((test-users/user->client :rasta) :post 202 "card"
+ ((mt/user->client :rasta) :post 202 "card"
(merge (card-with-name-and-query "d")
{:collection_id (u/get-id collection)
:collection_position nil}))
@@ -924,9 +927,9 @@
Pulse e
Pulse f]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id d))
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id d))
{:collection_position 1, :collection_id (u/get-id collection)})
- (name->position ((test-users/user->client :rasta) :get 200 (format "collection/%s/items" (u/get-id collection))))))))))
+ (name->position ((mt/user->client :rasta) :get 200 (format "collection/%s/items" (u/get-id collection))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -946,23 +949,23 @@
[{:message "Archiving a Card should trigger Alert deletion"
:expected-email "the question was archived by Rasta Toucan"
:f (fn [{:keys [card]}]
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:archived true}))}
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:archived true}))}
{:message "Validate changing a display type triggers alert deletion"
:card {:display :table}
:expected-email "the question was edited by Rasta Toucan"
:f (fn [{:keys [card]}]
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :line}))}
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :line}))}
{:message "Changing the display type from line to table should force a delete"
:card {:display :line}
:expected-email "the question was edited by Rasta Toucan"
:f (fn [{:keys [card]}]
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :table}))}
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :table}))}
{:message "Removing the goal value will trigger the alert to be deleted"
:card {:display :line
:visualization_settings {:graph.goal_value 10}}
:expected-email "the question was edited by Rasta Toucan"
:f (fn [{:keys [card]}]
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:visualization_settings {:something "else"}}))}
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:visualization_settings {:something "else"}}))}
{:message "Adding an additional breakout will cause the alert to be removed"
:card {:display :line
:visualization_settings {:graph.goal_value 10}
@@ -974,7 +977,7 @@
"hour"]])}
:expected-email "the question was edited by Crowberto Corv"
:f (fn [{:keys [card]}]
- ((test-users/user->client :crowberto) :put 202 (str "card/" (u/get-id card))
+ ((mt/user->client :crowberto) :put 202 (str "card/" (u/get-id card))
{:dataset_query (assoc-in (mbql-count-query (data/id) (data/id :checkins))
[:query :breakout] [[:datetime-field (data/id :checkins :date) "hour"]
[:datetime-field (data/id :checkins :date) "minute"]])}))}]]
@@ -982,16 +985,16 @@
(tt/with-temp* [Card [card card]
Pulse [pulse {:alert_condition "rows"
:alert_first_only false
- :creator_id (test-users/user->id :rasta)
+ :creator_id (mt/user->id :rasta)
:name "Original Alert Name"}]
PulseCard [_ {:pulse_id (u/get-id pulse)
:card_id (u/get-id card)
:position 0}]
PulseChannel [pc {:pulse_id (u/get-id pulse)}]
- PulseChannelRecipient [_ {:user_id (test-users/user->id :crowberto)
+ PulseChannelRecipient [_ {:user_id (mt/user->id :crowberto)
:pulse_channel_id (u/get-id pc)}]
- PulseChannelRecipient [_ {:user_id (test-users/user->id :rasta)
+ PulseChannelRecipient [_ {:user_id (mt/user->id :rasta)
:pulse_channel_id (u/get-id pc)}]]
(with-cards-in-writeable-collection card
(et/with-fake-inbox
@@ -1015,23 +1018,23 @@
:visualization_settings {:graph.goal_value 10}}]
Pulse [pulse {:alert_condition "goal"
:alert_first_only false
- :creator_id (test-users/user->id :rasta)
+ :creator_id (mt/user->id :rasta)
:name "Original Alert Name"}]
PulseCard [_ {:pulse_id (u/get-id pulse)
:card_id (u/get-id card)
:position 0}]
PulseChannel [pc {:pulse_id (u/get-id pulse)}]
- PulseChannelRecipient [_ {:user_id (test-users/user->id :rasta)
+ PulseChannelRecipient [_ {:user_id (mt/user->id :rasta)
:pulse_channel_id (u/get-id pc)}]]
(with-cards-in-writeable-collection card
(et/with-fake-inbox
(array-map
:emails-1 (do
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :area})
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :area})
(et/regex-email-bodies #"the question was edited by Rasta Toucan"))
:pulse-1 (boolean (Pulse (u/get-id pulse)))
:emails-2 (do
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :bar})
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:display :bar})
(et/regex-email-bodies #"the question was edited by Rasta Toucan"))
:pulse-2 (boolean (Pulse (u/get-id pulse))))))))))
@@ -1043,13 +1046,13 @@
(deftest check-that-we-can-delete-a-card
(is (nil? (tt/with-temp Card [card]
(with-cards-in-writeable-collection card
- ((test-users/user->client :rasta) :delete 204 (str "card/" (u/get-id card)))
+ ((mt/user->client :rasta) :delete 204 (str "card/" (u/get-id card)))
(Card (u/get-id card)))))))
;; deleting a card that doesn't exist should return a 404 (#1957)
(deftest deleting-a-card-that-doesnt-exist-should-return-a-404---1957-
(is (= "Not found."
- ((test-users/user->client :crowberto) :delete 404 "card/12345"))))
+ ((mt/user->client :crowberto) :delete 404 "card/12345"))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -1058,13 +1061,13 @@
;; Helper Functions
(defn- fave? [card]
- (db/exists? CardFavorite, :card_id (u/get-id card), :owner_id (test-users/user->id :rasta)))
+ (db/exists? CardFavorite, :card_id (u/get-id card), :owner_id (mt/user->id :rasta)))
(defn- fave! [card]
- ((test-users/user->client :rasta) :post 200 (format "card/%d/favorite" (u/get-id card))))
+ ((mt/user->client :rasta) :post 200 (format "card/%d/favorite" (u/get-id card))))
(defn- unfave! [card]
- ((test-users/user->client :rasta) :delete 204 (format "card/%d/favorite" (u/get-id card))))
+ ((mt/user->client :rasta) :delete 204 (format "card/%d/favorite" (u/get-id card))))
;; ## GET /api/card/:id/favorite
(deftest can-we-see-if-a-card-is-a-favorite--
@@ -1115,7 +1118,7 @@
(is (= ["COUNT(*)"
"75"]
(str/split-lines
- ((test-users/user->client :rasta) :post 202 (format "card/%d/query/csv"
+ ((mt/user->client :rasta) :post 202 (format "card/%d/query/csv"
(u/get-id card)))))))))
(testing "with-paramters"
(with-temp-native-card-with-params [_ card]
@@ -1123,7 +1126,7 @@
(is (= ["COUNT(*)"
"8"]
(str/split-lines
- ((test-users/user->client :rasta) :post 202 (format "card/%d/query/csv?parameters=%s"
+ ((mt/user->client :rasta) :post 202 (format "card/%d/query/csv?parameters=%s"
(u/get-id card) encoded-params)))))))))
(deftest json-download-test
@@ -1131,12 +1134,12 @@
(with-temp-native-card [_ card]
(with-cards-in-readable-collection card
(is (= [{(keyword "COUNT(*)") 75}]
- ((test-users/user->client :rasta) :post 202 (format "card/%d/query/json" (u/get-id card))))))))
+ ((mt/user->client :rasta) :post 202 (format "card/%d/query/json" (u/get-id card))))))))
(testing "with parameters"
(with-temp-native-card-with-params [_ card]
(with-cards-in-readable-collection card
(is (= [{(keyword "COUNT(*)") 8}]
- ((test-users/user->client :rasta) :post 202 (format "card/%d/query/json?parameters=%s"
+ ((mt/user->client :rasta) :post 202 (format "card/%d/query/json?parameters=%s"
(u/get-id card) encoded-params))))))))
(defn- parse-xlsx-results [results]
@@ -1152,14 +1155,14 @@
(with-cards-in-readable-collection card
(is (= [{:col "COUNT(*)"} {:col 75.0}]
(parse-xlsx-results
- ((test-users/user->client :rasta) :post 202 (format "card/%d/query/xlsx" (u/get-id card))
+ ((mt/user->client :rasta) :post 202 (format "card/%d/query/xlsx" (u/get-id card))
{:request-options {:as :byte-array}})))))))
(testing "with parameters"
(with-temp-native-card-with-params [_ card]
(with-cards-in-readable-collection card
(is (= [{:col "COUNT(*)"} {:col 8.0}]
(parse-xlsx-results
- ((test-users/user->client :rasta) :post 202 (format "card/%d/query/xlsx?parameters=%s"
+ ((mt/user->client :rasta) :post 202 (format "card/%d/query/xlsx?parameters=%s"
(u/get-id card) encoded-params)
{:request-options {:as :byte-array}}))))))))
@@ -1178,18 +1181,18 @@
options))]
(testing "Sanity check: this CSV download should not be subject to C O N S T R A I N T S"
(is (= {:constraints nil}
- ((test-users/user->client :rasta) :post 200 (format "card/%d/query/csv" (u/get-id card))))))
+ ((mt/user->client :rasta) :post 200 (format "card/%d/query/csv" (u/get-id card))))))
(with-redefs [constraints/default-query-constraints {:max-results 10, :max-results-bare-rows 10}]
(testing (str "Downloading CSV/JSON/XLSX results shouldn't be subject to the default query constraints -- even "
"if the query comes in with `add-default-userland-constraints` (as will be the case if the query "
"gets saved from one that had it -- see #9831)")
(is (= {:constraints nil}
- ((test-users/user->client :rasta) :post 200 (format "card/%d/query/csv" (u/get-id card))))))
+ ((mt/user->client :rasta) :post 200 (format "card/%d/query/csv" (u/get-id card))))))
(testing (str "non-\"download\" queries should still get the default constraints (this also is a sanitiy "
"check to make sure the `with-redefs` in the test above actually works)")
(is (= {:constraints {:max-results 10, :max-results-bare-rows 10}}
- ((test-users/user->client :rasta) :post 200 (format "card/%d/query" (u/get-id card))))))))))))
+ ((mt/user->client :rasta) :post 200 (format "card/%d/query" (u/get-id card))))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -1201,7 +1204,7 @@
(tt/with-temp Collection [collection]
(tu/with-model-cleanup [Card]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (let [card ((test-users/user->client :rasta) :post 202 "card"
+ (let [card ((mt/user->client :rasta) :post 202 "card"
(assoc (card-with-name-and-query)
:collection_id (u/get-id collection)))]
(= (db/select-one-field :collection_id Card :id (u/get-id card))
@@ -1212,14 +1215,14 @@
(tu/with-non-admin-groups-no-root-collection-perms
(tu/with-model-cleanup [Card]
(tt/with-temp Collection [collection]
- ((test-users/user->client :rasta) :post 403 "card"
+ ((mt/user->client :rasta) :post 403 "card"
(assoc (card-with-name-and-query)
:collection_id (u/get-id collection)))))))))
(deftest make-sure-we-can-change-the--collection-id--of-a-card-if-it-s-not-in-any-collection
(tt/with-temp* [Card [card]
Collection [collection]]
- ((test-users/user->client :crowberto) :put 202 (str "card/" (u/get-id card)) {:collection_id (u/get-id collection)})
+ ((mt/user->client :crowberto) :put 202 (str "card/" (u/get-id card)) {:collection_id (u/get-id collection)})
(= (db/select-one-field :collection_id Card :id (u/get-id card))
(u/get-id collection))))
@@ -1228,7 +1231,7 @@
(tu/with-non-admin-groups-no-root-collection-perms
(tt/with-temp* [Collection [collection]
Card [card {:collection_id (u/get-id collection)}]]
- ((test-users/user->client :rasta) :put 403 (str "card/" (u/get-id card))
+ ((mt/user->client :rasta) :put 403 (str "card/" (u/get-id card))
{:name "Number of Blueberries Consumed Per Month"}))))))
@@ -1241,7 +1244,7 @@
Collection [new-collection]
Card [card {:collection_id (u/get-id original-collection)}]]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) original-collection)
- ((test-users/user->client :rasta) :put 403 (str "card/" (u/get-id card))
+ ((mt/user->client :rasta) :put 403 (str "card/" (u/get-id card))
{:collection_id (u/get-id new-collection)}))))))
@@ -1254,7 +1257,7 @@
Collection [new-collection]
Card [card {:collection_id (u/get-id original-collection)}]]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) new-collection)
- ((test-users/user->client :rasta) :put 403 (str "card/" (u/get-id card))
+ ((mt/user->client :rasta) :put 403 (str "card/" (u/get-id card))
{:collection_id (u/get-id new-collection)}))))))
@@ -1266,7 +1269,7 @@
Card [card {:collection_id (u/get-id original-collection)}]]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) original-collection)
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) new-collection)
- ((test-users/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:collection_id (u/get-id new-collection)})
+ ((mt/user->client :rasta) :put 202 (str "card/" (u/get-id card)) {:collection_id (u/get-id new-collection)})
(= (db/select-one-field :collection_id Card :id (u/get-id card))
(u/get-id new-collection)))))
@@ -1290,7 +1293,7 @@
[username expected-status-code collection-or-collection-id-or-nil cards-or-card-ids]
(array-map
:response
- ((test-users/user->client username) :post expected-status-code "card/collections"
+ ((mt/user->client username) :post expected-status-code "card/collections"
{:collection_id (when collection-or-collection-id-or-nil
(u/get-id collection-or-collection-id-or-nil))
:card_ids (map u/get-id cards-or-card-ids)})
@@ -1376,8 +1379,8 @@
Card [card-e {:name "e", :collection_id coll-id-2, :collection_position 2}]
Card [card-f {:name "f", :collection_id coll-id-2, :collection_position 3}]]
[(POST-card-collections! :crowberto 200 new-collection [card-a card-b])
- (merge (name->position ((test-users/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-1) :model "card" :archived "false"))
- (name->position ((test-users/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-2) :model "card" :archived "false")))]))))
+ (merge (name->position ((mt/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-1) :model "card" :archived "false"))
+ (name->position ((mt/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-2) :model "card" :archived "false")))]))))
(deftest moving-a-card-without-a-collection-position-keeps-the-collection-position-nil
(is (= [{:response {:status "ok"}
@@ -1392,8 +1395,8 @@
Card [card-b {:name "b", :collection_id coll-id-2, :collection_position 1}]
Card [card-c {:name "c", :collection_id coll-id-2, :collection_position 2}]]
[(POST-card-collections! :crowberto 200 new-collection [card-a card-b])
- (merge (name->position ((test-users/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-1) :model "card" :archived "false"))
- (name->position ((test-users/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-2) :model "card" :archived "false")))]))))
+ (merge (name->position ((mt/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-1) :model "card" :archived "false"))
+ (name->position ((mt/user->client :crowberto) :get 200 (format "collection/%s/items" coll-id-2) :model "card" :archived "false")))]))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | PUBLIC SHARING ENDPOINTS |
@@ -1401,7 +1404,7 @@
(defn- shared-card []
{:public_uuid (str (UUID/randomUUID))
- :made_public_by_id (test-users/user->id :crowberto)})
+ :made_public_by_id (mt/user->id :crowberto)})
;;; ----------------------------------------- POST /api/card/:id/public_link -----------------------------------------
@@ -1409,7 +1412,7 @@
(deftest test-that-we-can-share-a-card
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card]
- (let [{uuid :uuid} ((test-users/user->client :crowberto) :post 200 (format "card/%d/public_link" (u/get-id card)))]
+ (let [{uuid :uuid} ((mt/user->client :crowberto) :post 200 (format "card/%d/public_link" (u/get-id card)))]
(is (= true
(boolean (db/exists? Card :id (u/get-id card), :public_uuid uuid))))))))
@@ -1417,37 +1420,37 @@
(is (= "You don't have permissions to do that."
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card]
- ((test-users/user->client :rasta) :post 403 (format "card/%d/public_link" (u/get-id card))))))))
+ ((mt/user->client :rasta) :post 403 (format "card/%d/public_link" (u/get-id card))))))))
(deftest test-that-we--cannot--share-a-card-if-the-setting-is-disabled
(is (= "Public sharing is not enabled."
(tu/with-temporary-setting-values [enable-public-sharing false]
(tt/with-temp Card [card]
- ((test-users/user->client :crowberto) :post 400 (format "card/%d/public_link" (u/get-id card))))))))
+ ((mt/user->client :crowberto) :post 400 (format "card/%d/public_link" (u/get-id card))))))))
(deftest test-that-we--cannot--share-a-card-if-the-card-has-been-archived
(is (= {:message "The object has been archived.", :error_code "archived"}
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card {:archived true}]
- ((test-users/user->client :crowberto) :post 404 (format "card/%d/public_link" (u/get-id card))))))))
+ ((mt/user->client :crowberto) :post 404 (format "card/%d/public_link" (u/get-id card))))))))
(deftest test-that-we-get-a-404-if-the-card-doesnt-exist
(is (= "Not found."
(tu/with-temporary-setting-values [enable-public-sharing true]
- ((test-users/user->client :crowberto) :post 404 (format "card/%d/public_link" Integer/MAX_VALUE))))))
+ ((mt/user->client :crowberto) :post 404 (format "card/%d/public_link" Integer/MAX_VALUE))))))
(deftest test-that-if-a-card-has-already-been-shared-we-re-se-the-existing-uuid
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card (shared-card)]
(= (:public_uuid card)
- (:uuid ((test-users/user->client :crowberto) :post 200 (format "card/%d/public_link" (u/get-id card))))))))
+ (:uuid ((mt/user->client :crowberto) :post 200 (format "card/%d/public_link" (u/get-id card))))))))
;;; ---------------------------------------- DELETE /api/card/:id/public_link ----------------------------------------
(deftest test-that-we-can-unshare-a-card
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card (shared-card)]
- ((test-users/user->client :crowberto) :delete 204 (format "card/%d/public_link" (u/get-id card)))
+ ((mt/user->client :crowberto) :delete 204 (format "card/%d/public_link" (u/get-id card)))
(is (= false
(db/exists? Card :id (u/get-id card), :public_uuid (:public_uuid card)))))))
@@ -1455,34 +1458,34 @@
(is (= "You don't have permissions to do that."
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card (shared-card)]
- ((test-users/user->client :rasta) :delete 403 (format "card/%d/public_link" (u/get-id card))))))))
+ ((mt/user->client :rasta) :delete 403 (format "card/%d/public_link" (u/get-id card))))))))
(deftest test-that-we-get-a-404-if-card-isn-t-shared
(is (= "Not found."
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [card]
- ((test-users/user->client :crowberto) :delete 404 (format "card/%d/public_link" (u/get-id card))))))))
+ ((mt/user->client :crowberto) :delete 404 (format "card/%d/public_link" (u/get-id card))))))))
(deftest test-that-we-get-a-404-if-card-doesnt-exist
(is (= "Not found."
(tu/with-temporary-setting-values [enable-public-sharing true]
- ((test-users/user->client :crowberto) :delete 404 (format "card/%d/public_link" Integer/MAX_VALUE))))))
+ ((mt/user->client :crowberto) :delete 404 (format "card/%d/public_link" Integer/MAX_VALUE))))))
(deftest test-that-we-can-fetch-a-list-of-publicly-accessible-cards
- (is (= [{:name true, :id true, :public_uuid true}]
- (tu/with-temporary-setting-values [enable-public-sharing true]
- (tt/with-temp Card [card (shared-card)]
- (for [card ((test-users/user->client :crowberto) :get 200 "card/public")]
+ (tu/with-temporary-setting-values [enable-public-sharing true]
+ (tt/with-temp Card [card (shared-card)]
+ (is (= [{:name true, :id true, :public_uuid true}]
+ (for [card ((mt/user->client :crowberto) :get 200 "card/public")]
(m/map-vals boolean (select-keys card [:name :id :public_uuid]))))))))
(deftest test-that-we-can-fetch-a-list-of-embeddable-cards
- (is (= [{:name true, :id true}]
- (tu/with-temporary-setting-values [enable-embedding true]
- (tt/with-temp Card [card {:enable_embedding true}]
- (for [card ((test-users/user->client :crowberto) :get 200 "card/embeddable")]
+ (tu/with-temporary-setting-values [enable-embedding true]
+ (tt/with-temp Card [card {:enable_embedding true}]
+ (is (= [{:name true, :id true}]
+ (for [card ((mt/user->client :crowberto) :get 200 "card/embeddable")]
(m/map-vals boolean (select-keys card [:name :id]))))))))
(deftest test-related-recommended-entities
- (is (= #{:table :metrics :segments :dashboard-mates :similar-questions :canonical-metric :dashboards :collections}
- (tt/with-temp Card [card]
- (-> ((test-users/user->client :crowberto) :get 200 (format "card/%s/related" (u/get-id card))) keys set)))))
+ (tt/with-temp Card [card]
+ (is (= #{:table :metrics :segments :dashboard-mates :similar-questions :canonical-metric :dashboards :collections}
+ (-> ((mt/user->client :crowberto) :get 200 (format "card/%s/related" (u/get-id card))) keys set)))))
diff --git a/test/metabase/api/pulse_test.clj b/test/metabase/api/pulse_test.clj
index 8a81833c5ca807378980f839a20c59ad4485298c..c3293fd05e5949e33e218bfb77f821ec39a78d7a 100644
--- a/test/metabase/api/pulse_test.clj
+++ b/test/metabase/api/pulse_test.clj
@@ -1,10 +1,10 @@
(ns metabase.api.pulse-test
"Tests for /api/pulse endpoints."
(:require [clojure.test :refer :all]
- [expectations :refer [expect]]
[metabase
- [email-test :as et]
[http-client :as http]
+ [models :refer [Card Collection Dashboard Pulse PulseCard PulseChannel PulseChannelRecipient]]
+ [test :as mt]
[util :as u]]
[metabase.api
[card-test :as card-api-test]
@@ -12,25 +12,14 @@
[metabase.integrations.slack :as slack]
[metabase.middleware.util :as middleware.u]
[metabase.models
- [card :refer [Card]]
- [collection :refer [Collection]]
- [dashboard :refer [Dashboard]]
- [database :refer [Database]]
[permissions :as perms]
[permissions-group :as perms-group]
- [pulse :as pulse :refer [Pulse]]
- [pulse-card :refer [PulseCard]]
- [pulse-channel :refer [PulseChannel]]
- [pulse-channel-recipient :refer [PulseChannelRecipient]]
- [pulse-test :as pulse-test]
- [table :refer [Table]]]
- [metabase.test
- [data :as data]
- [util :as tu]]
- [metabase.test.data.users :refer :all]
+ [pulse :as pulse]
+ [pulse-test :as pulse-test]]
+ [metabase.pulse.render.png :as png]
[metabase.test.mock.util :refer [pulse-channel-defaults]]
- [toucan.db :as db]
- [toucan.util.test :as tt]))
+ [schema.core :as s]
+ [toucan.db :as db]))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Helper Fns & Macros |
@@ -68,8 +57,8 @@
(update card :collection_id boolean)))))
(defn- do-with-pulses-in-a-collection [grant-collection-perms-fn! pulses-or-ids f]
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [collection]
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (mt/with-temp Collection [collection]
(grant-collection-perms-fn! (perms-group/all-users) collection)
;; use db/execute! instead of db/update! so the updated_at field doesn't get automatically updated!
(when (seq pulses-or-ids)
@@ -92,8 +81,9 @@
;; We assume that all endpoints for a given context are enforced by the same middleware, so we don't run the same
;; authentication test on every single individual endpoint
-(expect (:body middleware.u/response-unauthentic) (http/client :get 401 "pulse"))
-(expect (:body middleware.u/response-unauthentic) (http/client :put 401 "pulse/13"))
+(deftest authentication-test
+ (is (= (:body middleware.u/response-unauthentic) (http/client :get 401 "pulse")))
+ (is (= (:body middleware.u/response-unauthentic) (http/client :put 401 "pulse/13"))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -107,43 +97,41 @@
"`(collection_id, description, display, id, include_csv, include_xls, name)` "
"2) value must be a map with the keys `id`, `include_csv`, and `include_xls`. The array cannot be empty.")}})
-(expect
- {:errors {:name "value must be a non-blank string."}}
- ((user->client :rasta) :post 400 "pulse" {}))
-
-(expect
- default-post-card-ref-validation-error
- ((user->client :rasta) :post 400 "pulse" {:name "abc"}))
-
-(expect
- default-post-card-ref-validation-error
- ((user->client :rasta) :post 400 "pulse" {:name "abc"
- :cards "foobar"}))
-
-(expect
- default-post-card-ref-validation-error
- ((user->client :rasta) :post 400 "pulse" {:name "abc"
- :cards ["abc"]}))
-
-(expect
- {:errors {:channels "value must be an array. Each value must be a map. The array cannot be empty."}}
- ((user->client :rasta) :post 400 "pulse" {:name "abc"
- :cards [{:id 100, :include_csv false, :include_xls false}
- {:id 200, :include_csv false, :include_xls false}]}))
-
-(expect
- {:errors {:channels "value must be an array. Each value must be a map. The array cannot be empty."}}
- ((user->client :rasta) :post 400 "pulse" {:name "abc"
- :cards [{:id 100, :include_csv false, :include_xls false}
- {:id 200, :include_csv false, :include_xls false}]
- :channels "foobar"}))
-
-(expect
- {:errors {:channels "value must be an array. Each value must be a map. The array cannot be empty."}}
- ((user->client :rasta) :post 400 "pulse" {:name "abc"
- :cards [{:id 100, :include_csv false, :include_xls false}
- {:id 200, :include_csv false, :include_xls false}]
- :channels ["abc"]}))
+(deftest create-pulse-validation-test
+ (doseq [[input expected-error]
+ {{}
+ {:errors {:name "value must be a non-blank string."}}
+
+ {:name "abc"}
+ default-post-card-ref-validation-error
+
+ {:name "abc"
+ :cards "foobar"}
+ default-post-card-ref-validation-error
+
+ {:name "abc"
+ :cards ["abc"]}
+ default-post-card-ref-validation-error
+
+ {:name "abc"
+ :cards [{:id 100, :include_csv false, :include_xls false}
+ {:id 200, :include_csv false, :include_xls false}]}
+ {:errors {:channels "value must be an array. Each value must be a map. The array cannot be empty."}}
+
+ {:name "abc"
+ :cards [{:id 100, :include_csv false, :include_xls false}
+ {:id 200, :include_csv false, :include_xls false}]
+ :channels "foobar"}
+ {:errors {:channels "value must be an array. Each value must be a map. The array cannot be empty."}}
+
+ {:name "abc"
+ :cards [{:id 100, :include_csv false, :include_xls false}
+ {:id 200, :include_csv false, :include_xls false}]
+ :channels ["abc"]}
+ {:errors {:channels "value must be an array. Each value must be a map. The array cannot be empty."}}}]
+ (testing (pr-str input)
+ (is (= expected-error
+ ((mt/user->client :rasta) :post 400 "pulse" input))))))
(defn- remove-extra-channels-fields [channels]
(for [channel channels]
@@ -165,49 +153,57 @@
:schedule_day nil
:recipients []})
-(tt/expect-with-temp [Card [card-1]
- Card [card-2]]
- (merge
- pulse-defaults
- {:name "A Pulse"
- :creator_id (user->id :rasta)
- :creator (user-details (fetch-user :rasta))
- :cards (for [card [card-1 card-2]]
- (assoc (pulse-card-details card)
- :collection_id true))
- :channels [(merge pulse-channel-defaults
- {:channel_type "email"
- :schedule_type "daily"
- :schedule_hour 12
- :recipients []})]
- :collection_id true})
- (card-api-test/with-cards-in-readable-collection [card-1 card-2]
- (tt/with-temp Collection [collection]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (tu/with-model-cleanup [Pulse]
- (-> ((user->client :rasta) :post 200 "pulse" {:name "A Pulse"
- :collection_id (u/get-id collection)
- :cards [{:id (u/get-id card-1)
- :include_csv false
- :include_xls false}
- {:id (u/get-id card-2)
- :include_csv false
- :include_xls false}]
- :channels [daily-email-channel]
- :skip_if_empty false})
- pulse-response
- (update :channels remove-extra-channels-fields))))))
-
-;; Create a pulse with a HybridPulseCard and a CardRef, PUT accepts this format, we should make sure POST does as well
-(tt/expect-with-temp [Card [card-1]
+(deftest create-test
+ (testing "POST /api/pulse"
+ (mt/with-temp* [Card [card-1]
+ Card [card-2]]
+ (card-api-test/with-cards-in-readable-collection [card-1 card-2]
+ (mt/with-temp Collection [collection]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ (mt/with-model-cleanup [Pulse]
+ (is (= (merge
+ pulse-defaults
+ {:name "A Pulse"
+ :creator_id (mt/user->id :rasta)
+ :creator (user-details (mt/fetch-user :rasta))
+ :cards (for [card [card-1 card-2]]
+ (assoc (pulse-card-details card)
+ :collection_id true))
+ :channels [(merge pulse-channel-defaults
+ {:channel_type "email"
+ :schedule_type "daily"
+ :schedule_hour 12
+ :recipients []})]
+ :collection_id true})
+ (-> ((mt/user->client :rasta) :post 200 "pulse" {:name "A Pulse"
+ :collection_id (u/get-id collection)
+ :cards [{:id (u/get-id card-1)
+ :include_csv false
+ :include_xls false}
+ {:id (u/get-id card-2)
+ :include_csv false
+ :include_xls false}]
+ :channels [daily-email-channel]
+ :skip_if_empty false})
+ pulse-response
+ (update :channels remove-extra-channels-fields))))))))))
+
+(deftest create-with-hybrid-pulse-card-test
+ (testing "POST /api/pulse"
+ (testing "Create a pulse with a HybridPulseCard and a CardRef, PUT accepts this format, we should make sure POST does as well"
+ (mt/with-temp* [Card [card-1]
Card [card-2 {:name "The card"
:description "Info"
:display :table}]]
- (merge
+ (card-api-test/with-cards-in-readable-collection [card-1 card-2]
+ (mt/with-temp Collection [collection]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ (mt/with-model-cleanup [Pulse]
+ (is (= (merge
pulse-defaults
{:name "A Pulse"
- :creator_id (user->id :rasta)
- :creator (user-details (fetch-user :rasta))
+ :creator_id (mt/user->id :rasta)
+ :creator (user-details (mt/fetch-user :rasta))
:cards (for [card [card-1 card-2]]
(assoc (pulse-card-details card)
:collection_id true))
@@ -217,96 +213,88 @@
:schedule_hour 12
:recipients []})]
:collection_id true})
- (card-api-test/with-cards-in-readable-collection [card-1 card-2]
- (tt/with-temp Collection [collection]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (tu/with-model-cleanup [Pulse]
- (-> ((user->client :rasta) :post 200 "pulse" {:name "A Pulse"
- :collection_id (u/get-id collection)
- :cards [{:id (u/get-id card-1)
- :include_csv false
- :include_xls false}
- (-> card-2
- (select-keys [:id :name :description :display :collection_id])
- (assoc :include_csv false, :include_xls false))]
- :channels [daily-email-channel]
- :skip_if_empty false})
- pulse-response
- (update :channels remove-extra-channels-fields))))))
-
-;; Create a pulse with a csv and xls
-(tt/expect-with-temp [Card [card-1]
+ (-> ((mt/user->client :rasta) :post 200 "pulse" {:name "A Pulse"
+ :collection_id (u/get-id collection)
+ :cards [{:id (u/get-id card-1)
+ :include_csv false
+ :include_xls false}
+ (-> card-2
+ (select-keys [:id :name :description :display :collection_id])
+ (assoc :include_csv false, :include_xls false))]
+ :channels [daily-email-channel]
+ :skip_if_empty false})
+ pulse-response
+ (update :channels remove-extra-channels-fields)))))))))))
+
+(deftest create-csv-xls-test
+ (testing "POST /api/pulse"
+ (testing "Create a pulse with a csv and xls"
+ (mt/with-temp* [Card [card-1]
Card [card-2]]
- (merge
- pulse-defaults
- {:name "A Pulse"
- :creator_id (user->id :rasta)
- :creator (user-details (fetch-user :rasta))
- :cards [(assoc (pulse-card-details card-1) :include_csv true, :include_xls true, :collection_id true)
- (assoc (pulse-card-details card-2) :collection_id true)]
- :channels [(merge pulse-channel-defaults
- {:channel_type "email"
- :schedule_type "daily"
- :schedule_hour 12
- :recipients []})]
- :collection_id true})
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [collection]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (tu/with-model-cleanup [Pulse]
- (card-api-test/with-cards-in-readable-collection [card-1 card-2]
- (-> ((user->client :rasta) :post 200 "pulse" {:name "A Pulse"
- :collection_id (u/get-id collection)
- :cards [{:id (u/get-id card-1)
- :include_csv true
- :include_xls true}
- {:id (u/get-id card-2)
- :include_csv false
- :include_xls false}]
- :channels [daily-email-channel]
- :skip_if_empty false})
- pulse-response
- (update :channels remove-extra-channels-fields)))))))
-
-;; Make sure we can create a Pulse with a Collection position
-(expect
- #metabase.models.pulse.PulseInstance{:collection_id true, :collection_position 1}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tu/with-model-cleanup [Pulse]
- (let [pulse-name (tu/random-name)]
- (tt/with-temp* [Card [card]
- Collection [collection]]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (card-api-test/with-cards-in-readable-collection [card]
- ((user->client :rasta) :post 200 "pulse" {:name pulse-name
- :cards [{:id (u/get-id card)
- :include_csv false
- :include_xls false}]
- :channels [daily-email-channel]
- :skip_if_empty false
- :collection_id (u/get-id collection)
- :collection_position 1})
- (some-> (db/select-one [Pulse :collection_id :collection_position] :name pulse-name)
- (update :collection_id (partial = (u/get-id collection))))))))))
-
-;; ...but not if we don't have permissions for the Collection
-(expect
- nil
- (tu/with-non-admin-groups-no-root-collection-perms
- (tu/with-model-cleanup [Pulse]
- (let [pulse-name (tu/random-name)]
- (tt/with-temp* [Card [card]
- Collection [collection]]
- ((user->client :rasta) :post 403 "pulse" {:name pulse-name
- :cards [{:id (u/get-id card)
- :include_csv false
- :include_xls false}]
- :channels [daily-email-channel]
- :skip_if_empty false
- :collection_id (u/get-id collection)
- :collection_position 1})
- (some-> (db/select-one [Pulse :collection_id :collection_position] :name pulse-name)
- (update :collection_id (partial = (u/get-id collection)))))))))
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (mt/with-temp Collection [collection]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ (mt/with-model-cleanup [Pulse]
+ (card-api-test/with-cards-in-readable-collection [card-1 card-2]
+ (is (= (merge
+ pulse-defaults
+ {:name "A Pulse"
+ :creator_id (mt/user->id :rasta)
+ :creator (user-details (mt/fetch-user :rasta))
+ :cards [(assoc (pulse-card-details card-1) :include_csv true, :include_xls true, :collection_id true)
+ (assoc (pulse-card-details card-2) :collection_id true)]
+ :channels [(merge pulse-channel-defaults
+ {:channel_type "email"
+ :schedule_type "daily"
+ :schedule_hour 12
+ :recipients []})]
+ :collection_id true})
+ (-> ((mt/user->client :rasta) :post 200 "pulse" {:name "A Pulse"
+ :collection_id (u/get-id collection)
+ :cards [{:id (u/get-id card-1)
+ :include_csv true
+ :include_xls true}
+ {:id (u/get-id card-2)
+ :include_csv false
+ :include_xls false}]
+ :channels [daily-email-channel]
+ :skip_if_empty false})
+ pulse-response
+ (update :channels remove-extra-channels-fields))))))))))))
+
+(deftest create-with-collection-position-test
+ (testing "POST /api/pulse"
+ (testing "Make sure we can create a Pulse with a Collection position"
+ (mt/with-model-cleanup [Pulse]
+ (letfn [(create-pulse! [expected-status-code pulse-name card collection]
+ (let [response ((mt/user->client :rasta) :post expected-status-code "pulse"
+ {:name pulse-name
+ :cards [{:id (u/get-id card)
+ :include_csv false
+ :include_xls false}]
+ :channels [daily-email-channel]
+ :skip_if_empty false
+ :collection_id (u/get-id collection)
+ :collection_position 1})]
+ (testing "response"
+ (is (= nil
+ (:errors response))))))]
+ (let [pulse-name (mt/random-name)]
+ (mt/with-temp* [Card [card]
+ Collection [collection]]
+ (card-api-test/with-cards-in-readable-collection [card]
+ (create-pulse! 200 pulse-name card collection)
+ (is (= {:collection_id (u/get-id collection), :collection_position 1}
+ (mt/derecordize (db/select-one [Pulse :collection_id :collection_position] :name pulse-name)))))))
+
+ (testing "...but not if we don't have permissions for the Collection"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (let [pulse-name (mt/random-name)]
+ (mt/with-temp* [Card [card]
+ Collection [collection]]
+ (create-pulse! 403 pulse-name card collection)
+ (is (= nil
+ (db/select-one [Pulse :collection_id :collection_position] :name pulse-name))))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -321,652 +309,607 @@
"`(collection_id, description, display, id, include_csv, include_xls, name)` "
"2) value must be a map with the keys `id`, `include_csv`, and `include_xls`. The array cannot be empty.")}})
-(expect
- {:errors {:name "value may be nil, or if non-nil, value must be a non-blank string."}}
- ((user->client :rasta) :put 400 "pulse/1" {:name 123}))
+(deftest update-pulse-validation-test
+ (testing "PUT /api/pulse/:id"
+ (doseq [[input expected-error]
+ {{:name 123}
+ {:errors {:name "value may be nil, or if non-nil, value must be a non-blank string."}}
-(expect
- default-put-card-ref-validation-error
- ((user->client :rasta) :put 400 "pulse/1" {:cards 123}))
+ {:cards 123}
+ default-put-card-ref-validation-error
-(expect
- default-put-card-ref-validation-error
- ((user->client :rasta) :put 400 "pulse/1" {:cards "foobar"}))
+ {:cards "foobar"}
+ default-put-card-ref-validation-error
-(expect
- default-put-card-ref-validation-error
- ((user->client :rasta) :put 400 "pulse/1" {:cards ["abc"]}))
+ {:cards ["abc"]}
+ default-put-card-ref-validation-error
-(expect
- {:errors {:channels (str "value may be nil, or if non-nil, value must be an array. Each value must be a map. "
- "The array cannot be empty.")}}
- ((user->client :rasta) :put 400 "pulse/1" {:channels 123}))
+ {:channels 123}
+ {:errors {:channels (str "value may be nil, or if non-nil, value must be an array. Each value must be a map. "
+ "The array cannot be empty.")}}
-(expect
- {:errors {:channels (str "value may be nil, or if non-nil, value must be an array. Each value must be a map. "
- "The array cannot be empty.")}}
- ((user->client :rasta) :put 400 "pulse/1" {:channels "foobar"}))
+ {:channels "foobar"}
+ {:errors {:channels (str "value may be nil, or if non-nil, value must be an array. Each value must be a map. "
+ "The array cannot be empty.")}}
-(expect
- {:errors {:channels (str "value may be nil, or if non-nil, value must be an array. Each value must be a map. "
- "The array cannot be empty.")}}
- ((user->client :rasta) :put 400 "pulse/1" {:channels ["abc"]}))
+ {:channels ["abc"]}
+ {:errors {:channels (str "value may be nil, or if non-nil, value must be an array. Each value must be a map. "
+ "The array cannot be empty.")}}}]
+ (testing (pr-str input)
+ (is (= expected-error
+ ((mt/user->client :rasta) :put 400 "pulse/1" input)))))))
-(tt/expect-with-temp [Pulse [pulse]
- PulseChannel [pc {:pulse_id (u/get-id pulse)}]
- PulseChannelRecipient [_ {:pulse_channel_id (u/get-id pc), :user_id (user->id :rasta)}]
- Card [card]]
- (merge
- pulse-defaults
- {:name "Updated Pulse"
- :creator_id (user->id :rasta)
- :creator (user-details (fetch-user :rasta))
- :cards [(assoc (pulse-card-details card)
- :collection_id true)]
- :channels [(merge pulse-channel-defaults
- {:channel_type "slack"
- :schedule_type "hourly"
- :details {:channels "#general"}
- :recipients []})]
- :collection_id true})
- (with-pulses-in-writeable-collection [pulse]
- (card-api-test/with-cards-in-readable-collection [card]
- (-> ((user->client :rasta) :put 200 (format "pulse/%d" (u/get-id pulse))
- {:name "Updated Pulse"
- :cards [{:id (u/get-id card)
- :include_csv false
- :include_xls false}]
- :channels [{:enabled true
- :channel_type "slack"
- :schedule_type "hourly"
- :schedule_hour 12
- :schedule_day "mon"
- :recipients []
- :details {:channels "#general"}}]
- :skip_if_empty false})
- pulse-response
- (update :channels remove-extra-channels-fields)))))
-
-;; Can we add a card to an existing pulse that has a card? Specifically this will include a HybridPulseCard (the
-;; original card associated with the pulse) and a CardRef (the new card)
-(tt/expect-with-temp [Pulse [pulse {:name "Original Pulse Name"}]
+(deftest update-test
+ (testing "PUT /api/pulse/:id"
+ (mt/with-temp* [Pulse [pulse]
+ PulseChannel [pc {:pulse_id (u/get-id pulse)}]
+ PulseChannelRecipient [_ {:pulse_channel_id (u/get-id pc), :user_id (mt/user->id :rasta)}]
+ Card [card]]
+ (with-pulses-in-writeable-collection [pulse]
+ (card-api-test/with-cards-in-readable-collection [card]
+ (is (= (merge
+ pulse-defaults
+ {:name "Updated Pulse"
+ :creator_id (mt/user->id :rasta)
+ :creator (user-details (mt/fetch-user :rasta))
+ :cards [(assoc (pulse-card-details card)
+ :collection_id true)]
+ :channels [(merge pulse-channel-defaults
+ {:channel_type "slack"
+ :schedule_type "hourly"
+ :details {:channels "#general"}
+ :recipients []})]
+ :collection_id true})
+ (-> ((mt/user->client :rasta) :put 200 (format "pulse/%d" (u/get-id pulse))
+ {:name "Updated Pulse"
+ :cards [{:id (u/get-id card)
+ :include_csv false
+ :include_xls false}]
+ :channels [{:enabled true
+ :channel_type "slack"
+ :schedule_type "hourly"
+ :schedule_hour 12
+ :schedule_day "mon"
+ :recipients []
+ :details {:channels "#general"}}]
+ :skip_if_empty false})
+ pulse-response
+ (update :channels remove-extra-channels-fields)))))))))
+
+(deftest add-card-to-existing-test
+ (testing "PUT /api/pulse/:id"
+ (testing "Can we add a card to an existing pulse that has a card?"
+ ;; Specifically this will include a HybridPulseCard (the original card associated with the pulse) and a CardRef
+ ;; (the new card)
+ (mt/with-temp* [Pulse [pulse {:name "Original Pulse Name"}]
Card [card-1 {:name "Test"
:description "Just Testing"}]
PulseCard [_ {:card_id (u/get-id card-1)
:pulse_id (u/get-id pulse)}]
Card [card-2 {:name "Test2"
:description "Just Testing2"}]]
- (merge
- pulse-defaults
- {:name "Original Pulse Name"
- :creator_id (user->id :rasta)
- :creator (user-details (fetch-user :rasta))
- :cards (mapv (comp #(assoc % :collection_id true) pulse-card-details) [card-1 card-2])
- :channels []
- :collection_id true})
- (with-pulses-in-writeable-collection [pulse]
- (card-api-test/with-cards-in-readable-collection [card-1 card-2]
- ;; The FE will include the original HybridPulseCard, similar to how the API returns the card via GET
- (let [pulse-cards (:cards ((user->client :rasta) :get 200 (format "pulse/%d" (u/get-id pulse))))]
- (-> ((user->client :rasta) :put 200 (format "pulse/%d" (u/get-id pulse))
- {:cards (concat pulse-cards
- [{:id (u/get-id card-2)
- :include_csv false
- :include_xls false}])})
- pulse-response
- (update :channels remove-extra-channels-fields))))))
-
-;; Can we update *just* the Collection ID of a Pulse?
-(expect
- (tt/with-temp* [Pulse [pulse]
- Collection [collection]]
- ((user->client :crowberto) :put 200 (str "pulse/" (u/get-id pulse))
- {:collection_id (u/get-id collection)})
- (= (db/select-one-field :collection_id Pulse :id (u/get-id pulse))
- (u/get-id collection))))
-
-;; Can we change the Collection a Pulse is in (assuming we have the permissions to do so)?
-(expect
- (pulse-test/with-pulse-in-collection [db collection pulse]
- (tt/with-temp Collection [new-collection]
- ;; grant Permissions for both new and old collections
- (doseq [coll [collection new-collection]]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) coll))
- ;; now make an API call to move collections
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse)) {:collection_id (u/get-id new-collection)})
- ;; Check to make sure the ID has changed in the DB
- (= (db/select-one-field :collection_id Pulse :id (u/get-id pulse))
- (u/get-id new-collection)))))
-
-;; ...but if we don't have the Permissions for the old collection, we should get an Exception
-(expect
- "You don't have permissions to do that."
- (pulse-test/with-pulse-in-collection [db collection pulse]
- (tt/with-temp Collection [new-collection]
- ;; grant Permissions for only the *new* collection
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) new-collection)
- ;; now make an API call to move collections. Should fail
- ((user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse)) {:collection_id (u/get-id new-collection)}))))
-
-;; ...and if we don't have the Permissions for the new collection, we should get an Exception
-(expect
- "You don't have permissions to do that."
- (pulse-test/with-pulse-in-collection [db collection pulse]
- (tt/with-temp Collection [new-collection]
- ;; grant Permissions for only the *old* collection
+ (with-pulses-in-writeable-collection [pulse]
+ (card-api-test/with-cards-in-readable-collection [card-1 card-2]
+ ;; The FE will include the original HybridPulseCard, similar to how the API returns the card via GET
+ (let [pulse-cards (:cards ((mt/user->client :rasta) :get 200 (format "pulse/%d" (u/get-id pulse))))]
+ (is (= (merge
+ pulse-defaults
+ {:name "Original Pulse Name"
+ :creator_id (mt/user->id :rasta)
+ :creator (user-details (mt/fetch-user :rasta))
+ :cards (mapv (comp #(assoc % :collection_id true) pulse-card-details) [card-1 card-2])
+ :channels []
+ :collection_id true})
+ (-> ((mt/user->client :rasta) :put 200 (format "pulse/%d" (u/get-id pulse))
+ {:cards (concat pulse-cards
+ [{:id (u/get-id card-2)
+ :include_csv false
+ :include_xls false}])})
+ pulse-response
+ (update :channels remove-extra-channels-fields)))))))))))
+
+(deftest update-collection-id-test
+ (testing "Can we update *just* the Collection ID of a Pulse?"
+ (mt/with-temp* [Pulse [pulse]
+ Collection [collection]]
+ ((mt/user->client :crowberto) :put 200 (str "pulse/" (u/get-id pulse))
+ {:collection_id (u/get-id collection)})
+ (is (= (db/select-one-field :collection_id Pulse :id (u/get-id pulse))
+ (u/get-id collection))))))
+
+(deftest change-collection-test
+ (testing "Can we change the Collection a Pulse is in (assuming we have the permissions to do so)?"
+ (pulse-test/with-pulse-in-collection [db collection pulse]
+ (mt/with-temp Collection [new-collection]
+ ;; grant Permissions for both new and old collections
+ (doseq [coll [collection new-collection]]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) coll))
+ ;; now make an API call to move collections
+ ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse)) {:collection_id (u/get-id new-collection)})
+ ;; Check to make sure the ID has changed in the DB
+ (is (= (db/select-one-field :collection_id Pulse :id (u/get-id pulse))
+ (u/get-id new-collection)))))
+
+ (testing "...but if we don't have the Permissions for the old collection, we should get an Exception"
+ (pulse-test/with-pulse-in-collection [db collection pulse]
+ (mt/with-temp Collection [new-collection]
+ ;; grant Permissions for only the *new* collection
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) new-collection)
+ ;; now make an API call to move collections. Should fail
+ (is (= "You don't have permissions to do that."
+ ((mt/user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse)) {:collection_id (u/get-id new-collection)}))))))
+
+ (testing "...and if we don't have the Permissions for the new collection, we should get an Exception"
+ (pulse-test/with-pulse-in-collection [db collection pulse]
+ (mt/with-temp Collection [new-collection]
+ ;; grant Permissions for only the *old* collection
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ ;; now make an API call to move collections. Should fail
+ (is (= "You don't have permissions to do that."
+ ((mt/user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse)) {:collection_id (u/get-id new-collection)}))))))))
+
+(deftest update-collection-position-test
+ (testing "Can we change the Collection position of a Pulse?"
+ (pulse-test/with-pulse-in-collection [_ collection pulse]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ;; now make an API call to move collections. Should fail
- ((user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse)) {:collection_id (u/get-id new-collection)}))))
-
-;; Can we change the Collection position of a Pulse?
-(expect
- 1
- (pulse-test/with-pulse-in-collection [_ collection pulse]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
- {:collection_position 1})
- (db/select-one-field :collection_position Pulse :id (u/get-id pulse))))
-
-;; ...and unset (unpin) it as well?
-(expect
- nil
- (pulse-test/with-pulse-in-collection [_ collection pulse]
- (db/update! Pulse (u/get-id pulse) :collection_position 1)
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
- {:collection_position nil})
- (db/select-one-field :collection_position Pulse :id (u/get-id pulse))))
-
-;; ...we shouldn't be able to if we don't have permissions for the Collection
-(expect
- nil
- (pulse-test/with-pulse-in-collection [_ collection pulse]
- ((user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse))
- {:collection_position 1})
- (db/select-one-field :collection_position Pulse :id (u/get-id pulse))))
-
-(expect
- 1
- (pulse-test/with-pulse-in-collection [_ collection pulse]
- (db/update! Pulse (u/get-id pulse) :collection_position 1)
- ((user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse))
- {:collection_position nil})
- (db/select-one-field :collection_position Pulse :id (u/get-id pulse))))
-
-;; Can we archive a Pulse?
-(expect
- (pulse-test/with-pulse-in-collection [_ collection pulse]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
- {:archived true})
- (db/select-one-field :archived Pulse :id (u/get-id pulse))))
-
-;; Can we unarchive a Pulse?
-(expect
- false
- (pulse-test/with-pulse-in-collection [_ collection pulse]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- (db/update! Pulse (u/get-id pulse) :archived true)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
- {:archived false})
- (db/select-one-field :archived Pulse :id (u/get-id pulse))))
-
-;; Does unarchiving a Pulse affect its Cards & Recipients? It shouldn't. This should behave as a PATCH-style endpoint!
-(expect
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [collection]
- Pulse [pulse {:collection_id (u/get-id collection)}]
- PulseChannel [pc {:pulse_id (u/get-id pulse)}]
- PulseChannelRecipient [pcr {:pulse_channel_id (u/get-id pc), :user_id (user->id :rasta)}]
- Card [card]]
+ ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
+ {:collection_position 1})
+ (is (= 1
+ (db/select-one-field :collection_position Pulse :id (u/get-id pulse)))))
+
+ (testing "...and unset (unpin) it as well?"
+ (pulse-test/with-pulse-in-collection [_ collection pulse]
+ (db/update! Pulse (u/get-id pulse) :collection_position 1)
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
+ {:collection_position nil})
+ (is (= nil
+ (db/select-one-field :collection_position Pulse :id (u/get-id pulse))))))
+
+ (testing "...we shouldn't be able to if we don't have permissions for the Collection"
+ (pulse-test/with-pulse-in-collection [_ collection pulse]
+ ((mt/user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse))
+ {:collection_position 1})
+ (is (= nil
+ (db/select-one-field :collection_position Pulse :id (u/get-id pulse))))
+
+ (testing "shouldn't be able to unset (unpin) a Pulse"
+ (db/update! Pulse (u/get-id pulse) :collection_position 1)
+ ((mt/user->client :rasta) :put 403 (str "pulse/" (u/get-id pulse))
+ {:collection_position nil})
+ (is (= 1
+ (db/select-one-field :collection_position Pulse :id (u/get-id pulse)))))))))
+
+(deftest archive-test
+ (testing "Can we archive a Pulse?"
+ (pulse-test/with-pulse-in-collection [_ collection pulse]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
+ ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
{:archived true})
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
+ (is (= true
+ (db/select-one-field :archived Pulse :id (u/get-id pulse)))))))
+
+(deftest unarchive-test
+ (testing "Can we unarchive a Pulse?"
+ (pulse-test/with-pulse-in-collection [_ collection pulse]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ (db/update! Pulse (u/get-id pulse) :archived true)
+ ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
{:archived false})
- (and (db/exists? PulseChannel :id (u/get-id pc))
- (db/exists? PulseChannelRecipient :id (u/get-id pcr))))))
+ (is (= false
+ (db/select-one-field :archived Pulse :id (u/get-id pulse))))))
+
+ (testing "Does unarchiving a Pulse affect its Cards & Recipients? It shouldn't. This should behave as a PATCH-style endpoint!"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (mt/with-temp* [Collection [collection]
+ Pulse [pulse {:collection_id (u/get-id collection)}]
+ PulseChannel [pc {:pulse_id (u/get-id pulse)}]
+ PulseChannelRecipient [pcr {:pulse_channel_id (u/get-id pc), :user_id (mt/user->id :rasta)}]
+ Card [card]]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
+ {:archived true})
+ ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
+ {:archived false})
+ (is (db/exists? PulseChannel :id (u/get-id pc)))
+ (is (db/exists? PulseChannelRecipient :id (u/get-id pcr)))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | UPDATING PULSE COLLECTION POSITIONS |
;;; +----------------------------------------------------------------------------------------------------------------+
-;; Check that we can update a pulse's position in a collection only pulses
-(expect
- {"d" 1
- "a" 2
- "b" 3
- "c" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [{coll-id :id :as collection}]
- (card-api-test/with-ordered-items collection [Pulse a
- Pulse b
- Pulse c
- Pulse d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id d))
- {:collection_position 1})
- (card-api-test/get-name->collection-position :rasta coll-id)))))
-
-;; Change the position of b to 4, will dec c and d
-(expect
- {"a" 1
- "c" 2
- "d" 3
- "b" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [{coll-id :id :as collection}]
- (card-api-test/with-ordered-items collection [Card a
- Pulse b
- Card c
- Dashboard d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id b))
- {:collection_position 4})
- (card-api-test/get-name->collection-position :rasta coll-id)))))
-
-;; Change the position of d to the 2, should inc b and c
-(expect
- {"a" 1
- "d" 2
- "b" 3
- "c" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [{coll-id :id :as collection}]
- (card-api-test/with-ordered-items collection [Card a
- Card b
- Dashboard c
- Pulse d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id d))
- {:collection_position 2})
- (card-api-test/get-name->collection-position :rasta coll-id)))))
-
-;; Change the position of a to the 4th, will decrement all existing items
-(expect
- {"b" 1
- "c" 2
- "d" 3
- "a" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [{coll-id :id :as collection}]
- (card-api-test/with-ordered-items collection [Pulse a
- Dashboard b
- Card c
- Pulse d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id a))
- {:collection_position 4})
- (card-api-test/get-name->collection-position :rasta coll-id)))))
-
-;; Change the position of the d to the 1st, will increment all existing items
-(expect
- {"d" 1
- "a" 2
- "b" 3
- "c" 4}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp Collection [{coll-id :id :as collection}]
- (card-api-test/with-ordered-items collection [Dashboard a
- Dashboard b
- Card c
- Pulse d]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id d))
- {:collection_position 1})
- (card-api-test/get-name->collection-position :rasta coll-id)))))
-
-;; Check that no position change, but changing collections still triggers a fixup of both collections
-;; Moving `c` from collection-1 to collection-2, `c` is now at position 3 in collection 2
-(expect
- [{"a" 1
- "b" 2
- "d" 3}
- {"e" 1
- "f" 2
- "c" 3
- "g" 4
- "h" 5}]
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [collection-1]
- Collection [collection-2]]
- (card-api-test/with-ordered-items collection-1 [Pulse a
- Card b
- Pulse c
- Dashboard d]
- (card-api-test/with-ordered-items collection-2 [Card e
- Card f
- Dashboard g
- Dashboard h]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-1)
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-2)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id c))
- {:collection_id (u/get-id collection-2)})
- [(card-api-test/get-name->collection-position :rasta (u/get-id collection-1))
- (card-api-test/get-name->collection-position :rasta (u/get-id collection-2))])))))
-
-;; Check that moving a pulse to another collection, with a changed position will fixup both collections
-;; Moving `b` to collection 2, giving it a position of 1
-(expect
- [{"a" 1
- "c" 2
- "d" 3}
- {"b" 1
- "e" 2
- "f" 3
- "g" 4
- "h" 5}]
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [collection-1]
- Collection [collection-2]]
- (card-api-test/with-ordered-items collection-1 [Pulse a
- Pulse b
- Dashboard c
- Card d]
- (card-api-test/with-ordered-items collection-2 [Card e
- Card f
- Pulse g
- Dashboard h]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-1)
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-2)
- ((user->client :rasta) :put 200 (str "pulse/" (u/get-id b))
- {:collection_id (u/get-id collection-2), :collection_position 1})
- [(card-api-test/get-name->collection-position :rasta (u/get-id collection-1))
- (card-api-test/get-name->collection-position :rasta (u/get-id collection-2))])))))
-
-;; Add a new pulse at position 2, causing existing pulses to be incremented
-(expect
- [{"a" 1
- "c" 2
- "d" 3}
- {"a" 1
- "b" 2
- "c" 3
- "d" 4}]
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [{coll-id :id :as collection}]
- Card [card-1]]
- (card-api-test/with-cards-in-readable-collection [card-1]
- (card-api-test/with-ordered-items collection [Card a
- Dashboard c
- Pulse d]
- (tu/with-model-cleanup [Pulse]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- [(card-api-test/get-name->collection-position :rasta coll-id)
- (do ((user->client :rasta) :post 200 "pulse" {:name "b"
- :collection_id (u/get-id collection)
- :cards [{:id (u/get-id card-1)
- :include_csv false
- :include_xls false}]
- :channels [daily-email-channel]
- :skip_if_empty false
- :collection_position 2})
- (card-api-test/get-name->collection-position :rasta coll-id))]))))))
-
-;; Add a new pulse without a position, should leave existing positions unchanged
-(expect
- [{"a" 1
- "c" 2
- "d" 3}
- {"a" 1
- "b" nil
- "c" 2
- "d" 3}]
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [{coll-id :id :as collection}]
- Card [card-1]]
- (card-api-test/with-cards-in-readable-collection [card-1]
- (card-api-test/with-ordered-items collection [Pulse a
- Card c
- Dashboard d]
- (tu/with-model-cleanup [Pulse]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- [(card-api-test/get-name->collection-position :rasta coll-id)
- (do ((user->client :rasta) :post 200 "pulse" {:name "b"
- :collection_id (u/get-id collection)
- :cards [{:id (u/get-id card-1)
- :include_csv false
- :include_xls false}]
- :channels [daily-email-channel]
- :skip_if_empty false})
- (card-api-test/get-name->collection-position :rasta coll-id))]))))))
+(defmulti ^:private move-pulse-test-action
+ {:arglists '([action context & args])}
+ (fn [action & _]
+ action))
+
+(defmethod move-pulse-test-action :move
+ [_ context pulse & {:keys [collection position]}]
+ (let [pulse (get-in context [:pulse pulse])
+ response ((mt/user->client :rasta) :put 200 (str "pulse/" (u/get-id pulse))
+ (merge
+ (when collection
+ {:collection_id (u/get-id (get-in context [:collection collection]))})
+ (when position
+ {:collection_position position})))]
+ (is (= nil
+ (:errors response)))))
+
+(defmethod move-pulse-test-action :insert-pulse
+ [_ context collection & {:keys [position]}]
+ (let [collection (get-in context [:collection collection])
+ response ((mt/user->client :rasta) :post 200 "pulse"
+ (merge
+ {:name "x"
+ :collection_id (u/get-id collection)
+ :cards [{:id (u/get-id (get-in context [:card 1]))
+ :include_csv false
+ :include_xls false}]
+ :channels [daily-email-channel]
+ :skip_if_empty false}
+ (when position
+ {:collection_position position})))]
+ (is (= nil
+ (:errors response)))))
+
+(def ^:private move-test-definitions
+ [{:message "Check that we can update a Pulse's position in a Collection"
+ :action [:move :d :position 1]
+ :expected {"d" 1
+ "a" 2
+ "b" 3
+ "c" 4}}
+ {:message "Change the position of b to 4, will dec c and d"
+ :action [:move :b :position 4]
+ :expected {"a" 1
+ "c" 2
+ "d" 3
+ "b" 4}}
+ {:message "Change the position of d to 2, should inc b and c"
+ :action [:move :d :position 2]
+ :expected {"a" 1
+ "d" 2
+ "b" 3
+ "c" 4}}
+ {:message "Change the position of a to 4th, will decrement all existing items"
+ :action [:move :a :position 4]
+ :expected {"b" 1
+ "c" 2
+ "d" 3
+ "a" 4}}
+ {:message "Change the position of the d to the 1st, will increment all existing items"
+ :action [:move :d :position 1]
+ :expected {"d" 1
+ "a" 2
+ "b" 3
+ "c" 4}}
+ {:message (str "Check that no position change, but changing collections still triggers a fixup of both "
+ "collections Moving `c` from collection-1 to collection-2, `c` is now at position 3 in "
+ "collection 2")
+ :action [:move :c :collection 2]
+ :expected [{"a" 1
+ "b" 2
+ "d" 3}
+ {"e" 1
+ "f" 2
+ "c" 3
+ "g" 4
+ "h" 5}]}
+ {:message (str "Check that moving a pulse to another collection, with a changed position will fixup "
+ "both collections Moving `b` to collection 2, giving it a position of 1")
+ :action [:move :b :collection 2, :position 1]
+ :expected [{"a" 1
+ "c" 2
+ "d" 3}
+ {"b" 1
+ "e" 2
+ "f" 3
+ "g" 4
+ "h" 5}]}
+ {:message "Add a new pulse at position 2, causing existing pulses to be incremented"
+ :action [:insert-pulse 1 :position 2]
+ :expected {"a" 1
+ "x" 2
+ "b" 3
+ "c" 4
+ "d" 5}}
+
+ {:message "Add a new pulse without a position, should leave existing positions unchanged"
+ :action [:insert-pulse 1]
+ :expected {"x" nil
+ "a" 1
+ "b" 2
+ "c" 3
+ "d" 4}}])
+
+(deftest move-pulse-test
+ (testing "PUT /api/pulse/:id"
+ (doseq [{:keys [message action expected]} move-test-definitions
+ :let [expected (if (map? expected) [expected] expected)]]
+ (testing (str "\n" message)
+ (mt/with-temp* [Collection [collection-1]
+ Collection [collection-2]
+ Card [card-1]]
+ (card-api-test/with-ordered-items collection-1 [Pulse a
+ Pulse b
+ Pulse c
+ Pulse d]
+ (card-api-test/with-ordered-items collection-2 [Card e
+ Card f
+ Dashboard g
+ Dashboard h]
+ (let [[action & args] action
+ context {:pulse {:a a, :b b, :c c, :d d, :e e, :f f, :g g, :h h}
+ :collection {1 collection-1, 2 collection-2}
+ :card {1 card-1}}]
+ (testing (str "\n" (pr-str (cons action args)))
+ (apply move-pulse-test-action action context args)))
+ (testing "\nPositions after actions for"
+ (testing "Collection 1"
+ (is (= (first expected)
+ (card-api-test/get-name->collection-position :rasta (u/get-id collection-1)))))
+ (when (second expected)
+ (testing "Collection 2"
+ (is (= (second expected)
+ (card-api-test/get-name->collection-position :rasta (u/get-id collection-2))))))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | DELETE /api/pulse/:id |
;;; +----------------------------------------------------------------------------------------------------------------+
-;; check that a regular user can delete a Pulse if they have write permissions for its collection (!)
-(expect
- nil
- (tt/with-temp* [Pulse [pulse]
- PulseChannel [pc {:pulse_id (u/get-id pulse)}]
- PulseChannelRecipient [_ {:pulse_channel_id (u/get-id pc), :user_id (user->id :rasta)}]]
- (with-pulses-in-writeable-collection [pulse]
- ((user->client :rasta) :delete 204 (format "pulse/%d" (u/get-id pulse)))
- (pulse/retrieve-pulse (u/get-id pulse)))))
-
-;; Check that a rando (e.g. someone without collection write access) isn't allowed to delete a pulse
-(expect
- "You don't have permissions to do that."
- (tt/with-temp* [Database [db (select-keys (data/db) [:engine :details])]
- Table [table (-> (Table (data/id :venues))
- (dissoc :id)
- (assoc :db_id (u/get-id db)))]
- Card [card {:dataset_query {:database (u/get-id db)
- :type "query"
- :query {:source-table (u/get-id table)
- :aggregation [[:count]]}}}]
- Pulse [pulse {:name "Daily Sad Toucans"}]
- PulseCard [_ {:pulse_id (u/get-id pulse), :card_id (u/get-id card)}]]
- (with-pulses-in-readable-collection [pulse]
- ;; revoke permissions for default group to this database
- (perms/revoke-permissions! (perms-group/all-users) (u/get-id db))
- ;; now a user without permissions to the Card in question should *not* be allowed to delete the pulse
- ((user->client :rasta) :delete 403 (format "pulse/%d" (u/get-id pulse))))))
+(deftest delete-test
+ (testing "DELETE /api/pulse/:id"
+ (testing "check that a regular user can delete a Pulse if they have write permissions for its collection (!)"
+ (mt/with-temp* [Pulse [pulse]
+ PulseChannel [pc {:pulse_id (u/get-id pulse)}]
+ PulseChannelRecipient [_ {:pulse_channel_id (u/get-id pc), :user_id (mt/user->id :rasta)}]]
+ (with-pulses-in-writeable-collection [pulse]
+ ((mt/user->client :rasta) :delete 204 (format "pulse/%d" (u/get-id pulse)))
+ (is (= nil
+ (pulse/retrieve-pulse (u/get-id pulse)))))))
+
+ (testing "check that a rando (e.g. someone without collection write access) isn't allowed to delete a pulse"
+ (mt/with-temp-copy-of-db
+ (mt/with-temp* [Card [card {:dataset_query {:database (mt/id)
+ :type "query"
+ :query {:source-table (mt/id :venues)
+ :aggregation [[:count]]}}}]
+ Pulse [pulse {:name "Daily Sad Toucans"}]
+ PulseCard [_ {:pulse_id (u/get-id pulse), :card_id (u/get-id card)}]]
+ (with-pulses-in-readable-collection [pulse]
+ ;; revoke permissions for default group to this database
+ (perms/revoke-permissions! (perms-group/all-users) (mt/id))
+ ;; now a user without permissions to the Card in question should *not* be allowed to delete the pulse
+ (is (= "You don't have permissions to do that."
+ ((mt/user->client :rasta) :delete 403 (format "pulse/%d" (u/get-id pulse)))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GET /api/pulse |
;;; +----------------------------------------------------------------------------------------------------------------+
-;; should come back in alphabetical order
-(tt/expect-with-temp [Pulse [pulse-1 {:name "ABCDEF"}]
- Pulse [pulse-2 {:name "GHIJKL"}]]
- [(assoc (pulse-details pulse-1) :can_write false, :collection_id true)
- (assoc (pulse-details pulse-2) :can_write false, :collection_id true)]
- (with-pulses-in-readable-collection [pulse-1 pulse-2]
- ;; delete anything else in DB just to be sure; this step may not be necessary any more
- (db/delete! Pulse :id [:not-in #{(u/get-id pulse-1)
- (u/get-id pulse-2)}])
- (for [pulse ((user->client :rasta) :get 200 "pulse")]
- (update pulse :collection_id boolean))))
-
-;; `can_write` property should get updated correctly based on whether current user can write
-(tt/expect-with-temp [Pulse [pulse-1 {:name "ABCDEF"}]
- Pulse [pulse-2 {:name "GHIJKL"}]]
- [(assoc (pulse-details pulse-1) :can_write true)
- (assoc (pulse-details pulse-2) :can_write true)]
- (do
- ;; delete anything else in DB just to be sure; this step may not be necessary any more
- (db/delete! Pulse :id [:not-in #{(u/get-id pulse-1)
- (u/get-id pulse-2)}])
- ((user->client :crowberto) :get 200 "pulse")))
-
-;; should not return alerts
-(tt/expect-with-temp [Pulse [pulse-1 {:name "ABCDEF"}]
+(deftest list-test
+ (testing "GET /api/pulse"
+ (mt/with-temp* [Pulse [pulse-1 {:name "ABCDEF"}]
+ Pulse [pulse-2 {:name "GHIJKL"}]]
+ (testing "should come back in alphabetical order"
+ (with-pulses-in-readable-collection [pulse-1 pulse-2]
+ ;; delete anything else in DB just to be sure; this step may not be necessary any more
+ (db/delete! Pulse :id [:not-in #{(u/get-id pulse-1)
+ (u/get-id pulse-2)}])
+ (is (= [(assoc (pulse-details pulse-1) :can_write false, :collection_id true)
+ (assoc (pulse-details pulse-2) :can_write false, :collection_id true)]
+ (for [pulse ((mt/user->client :rasta) :get 200 "pulse")]
+ (update pulse :collection_id boolean))))))
+
+ (testing "`can_write` property should get updated correctly based on whether current user can write"
+ ;; delete anything else in DB just to be sure; this step may not be necessary any more
+ (db/delete! Pulse :id [:not-in #{(u/get-id pulse-1)
+ (u/get-id pulse-2)}])
+ (is (= [(assoc (pulse-details pulse-1) :can_write true)
+ (assoc (pulse-details pulse-2) :can_write true)]
+ ((mt/user->client :crowberto) :get 200 "pulse")))))
+
+ (testing "should not return alerts"
+ (mt/with-temp* [Pulse [pulse-1 {:name "ABCDEF"}]
Pulse [pulse-2 {:name "GHIJKL"}]
Pulse [pulse-3 {:name "AAAAAA"
:alert_condition "rows"}]]
- [(assoc (pulse-details pulse-1) :can_write false, :collection_id true)
- (assoc (pulse-details pulse-2) :can_write false, :collection_id true)]
- (with-pulses-in-readable-collection [pulse-1 pulse-2 pulse-3]
- (for [pulse ((user->client :rasta) :get 200 "pulse")]
- (update pulse :collection_id boolean))))
-
-;; by default, archived Pulses should be excluded
-(expect
- #{"Not Archived"}
- (tt/with-temp* [Pulse [not-archived-pulse {:name "Not Archived"}]
- Pulse [archived-pulse {:name "Archived", :archived true}]]
- (with-pulses-in-readable-collection [not-archived-pulse archived-pulse]
- (set (map :name ((user->client :rasta) :get 200 "pulse"))))))
-
-;; can we fetch archived Pulses?
-(expect
- #{"Archived"}
- (tt/with-temp* [Pulse [not-archived-pulse {:name "Not Archived"}]
- Pulse [archived-pulse {:name "Archived", :archived true}]]
- (with-pulses-in-readable-collection [not-archived-pulse archived-pulse]
- (set (map :name ((user->client :rasta) :get 200 "pulse?archived=true"))))))
+ (with-pulses-in-readable-collection [pulse-1 pulse-2 pulse-3]
+ (is (= [(assoc (pulse-details pulse-1) :can_write false, :collection_id true)
+ (assoc (pulse-details pulse-2) :can_write false, :collection_id true)]
+ (for [pulse ((mt/user->client :rasta) :get 200 "pulse")]
+ (update pulse :collection_id boolean)))))))
+
+ (testing "by default, archived Pulses should be excluded"
+ (mt/with-temp* [Pulse [not-archived-pulse {:name "Not Archived"}]
+ Pulse [archived-pulse {:name "Archived", :archived true}]]
+ (with-pulses-in-readable-collection [not-archived-pulse archived-pulse]
+ (is (= #{"Not Archived"}
+ (set (map :name ((mt/user->client :rasta) :get 200 "pulse"))))))))
+
+ (testing "can we fetch archived Pulses?"
+ (mt/with-temp* [Pulse [not-archived-pulse {:name "Not Archived"}]
+ Pulse [archived-pulse {:name "Archived", :archived true}]]
+ (with-pulses-in-readable-collection [not-archived-pulse archived-pulse]
+ (is (= #{"Archived"}
+ (set (map :name ((mt/user->client :rasta) :get 200 "pulse?archived=true"))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GET /api/pulse/:id |
;;; +----------------------------------------------------------------------------------------------------------------+
-(tt/expect-with-temp [Pulse [pulse]]
- (assoc (pulse-details pulse)
- :can_write false
- :collection_id true)
- (with-pulses-in-readable-collection [pulse]
- (-> ((user->client :rasta) :get 200 (str "pulse/" (u/get-id pulse)))
- (update :collection_id boolean))))
+(deftest get-pulse-test
+ (testing "GET /api/pulse/:id"
+ (mt/with-temp Pulse [pulse]
+ (with-pulses-in-readable-collection [pulse]
+ (is (= (assoc (pulse-details pulse)
+ :can_write false
+ :collection_id true)
+ (-> ((mt/user->client :rasta) :get 200 (str "pulse/" (u/get-id pulse)))
+ (update :collection_id boolean))))))
-;; Should 404 for an Alert
-(expect
- "Not found."
- (tt/with-temp Pulse [{pulse-id :id} {:alert_condition "rows"}]
- (with-pulses-in-readable-collection [pulse-id]
- ((user->client :rasta) :get 404 (str "pulse/" pulse-id)))))
+ (testing "should 404 for an Alert"
+ (mt/with-temp Pulse [{pulse-id :id} {:alert_condition "rows"}]
+ (is (= "Not found."
+ (with-pulses-in-readable-collection [pulse-id]
+ ((mt/user->client :rasta) :get 404 (str "pulse/" pulse-id)))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | POST /api/pulse/test |
;;; +----------------------------------------------------------------------------------------------------------------+
-(deftest create-pulse-test
- (tu/with-non-admin-groups-no-root-collection-perms
- (tu/with-model-cleanup [Pulse]
- (et/with-fake-inbox
- (data/dataset sad-toucan-incidents
- (tt/with-temp* [Collection [collection]
- Card [card {:dataset_query {:database (data/id)
- :type "query"
- :query {:source-table (data/id :incidents)
- :aggregation [[:count]]}}}]]
+(deftest send-test-pulse-test
+ (testing "POST /api/pulse/test"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (mt/with-fake-inbox
+ (mt/dataset sad-toucan-incidents
+ (mt/with-temp* [Collection [collection]
+ Card [card {:dataset_query (mt/mbql-query incidents {:aggregation [[:count]]})}]]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
(card-api-test/with-cards-in-readable-collection [card]
(is (= {:ok true}
- ((user->client :rasta) :post 200 "pulse/test" {:name "Daily Sad Toucans"
- :collection_id (u/get-id collection)
- :cards [{:id (u/get-id card)
- :include_csv false
- :include_xls false}]
- :channels [{:enabled true
- :channel_type "email"
- :schedule_type "daily"
- :schedule_hour 12
- :schedule_day nil
- :recipients [(fetch-user :rasta)]}]
- :skip_if_empty false})))
- (is (= (et/email-to :rasta {:subject "Pulse: Daily Sad Toucans"
+ ((mt/user->client :rasta) :post 200 "pulse/test" {:name "Daily Sad Toucans"
+ :collection_id (u/get-id collection)
+ :cards [{:id (u/get-id card)
+ :include_csv false
+ :include_xls false}]
+ :channels [{:enabled true
+ :channel_type "email"
+ :schedule_type "daily"
+ :schedule_hour 12
+ :schedule_day nil
+ :recipients [(mt/fetch-user :rasta)]}]
+ :skip_if_empty false})))
+ (is (= (mt/email-to :rasta {:subject "Pulse: Daily Sad Toucans"
:body {"Daily Sad Toucans" true}})
- (et/regex-email-bodies #"Daily Sad Toucans"))))))))))
+ (mt/regex-email-bodies #"Daily Sad Toucans"))))))))))
;; This test follows a flow that the user/UI would follow by first creating a pulse, then making a small change to
;; that pulse and testing it. The primary purpose of this test is to ensure tha the pulse/test endpoint accepts data
;; of the same format that the pulse GET returns
-(tt/expect-with-temp [Card [card-1 {:dataset_query
- {:database (data/id), :type :query, :query {:source-table (data/id :venues)}}}]
- Card [card-2 {:dataset_query
- {:database (data/id), :type :query, :query {:source-table (data/id :venues)}}}]]
- {:response {:ok true}
- :emails (et/email-to :rasta {:subject "Pulse: A Pulse"
- :body {"A Pulse" true}})}
- (card-api-test/with-cards-in-readable-collection [card-1 card-2]
- (et/with-fake-inbox
- (tu/with-model-cleanup [Pulse]
- (tt/with-temp Collection [collection]
- (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
- ;; First create the pulse
- (let [{pulse-id :id} ((user->client :rasta) :post 200 "pulse"
- {:name "A Pulse"
- :collection_id (u/get-id collection)
- :skip_if_empty false
- :cards [{:id (u/get-id card-1)
- :include_csv false
- :include_xls false}
- {:id (u/get-id card-2)
- :include_csv false
- :include_xls false}]
-
- :channels [(assoc daily-email-channel :recipients [(fetch-user :rasta)
- (fetch-user :crowberto)])]})
- ;; Retrieve the pulse via GET
- result ((user->client :rasta) :get 200 (str "pulse/" pulse-id))
- ;; Change our fetched copy of the pulse to only have Rasta for the recipients
- email-channel (assoc (-> result :channels first) :recipients [(fetch-user :rasta)])]
- ;; Don't update the pulse, but test the pulse with the updated recipients
- {:response ((user->client :rasta) :post 200 "pulse/test" (assoc result :channels [email-channel]))
- :emails (et/regex-email-bodies #"A Pulse")}))))))
-
-;; A Card saved with `:async?` true should not be ran async for a Pulse
-(expect
- map?
- (#'pulse-api/pulse-card-query-results
- {:id 1
- :dataset_query {:database (data/id)
- :type :query
- :query {:source-table (data/id :venues)
- :limit 1}
- :async? true}}))
+(deftest update-flow-test
+ (mt/with-temp* [Card [card-1 {:dataset_query
+ {:database (mt/id), :type :query, :query {:source-table (mt/id :venues)}}}]
+ Card [card-2 {:dataset_query
+ {:database (mt/id), :type :query, :query {:source-table (mt/id :venues)}}}]]
+
+ (card-api-test/with-cards-in-readable-collection [card-1 card-2]
+ (mt/with-fake-inbox
+ (mt/with-model-cleanup [Pulse]
+ (mt/with-temp Collection [collection]
+ (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
+ ;; First create the pulse
+ (let [{pulse-id :id} ((mt/user->client :rasta) :post 200 "pulse"
+ {:name "A Pulse"
+ :collection_id (u/get-id collection)
+ :skip_if_empty false
+ :cards [{:id (u/get-id card-1)
+ :include_csv false
+ :include_xls false}
+ {:id (u/get-id card-2)
+ :include_csv false
+ :include_xls false}]
+
+ :channels [(assoc daily-email-channel :recipients [(mt/fetch-user :rasta)
+ (mt/fetch-user :crowberto)])]})
+ ;; Retrieve the pulse via GET
+ result ((mt/user->client :rasta) :get 200 (str "pulse/" pulse-id))
+ ;; Change our fetched copy of the pulse to only have Rasta for the recipients
+ email-channel (assoc (-> result :channels first) :recipients [(mt/fetch-user :rasta)])]
+ ;; Don't update the pulse, but test the pulse with the updated recipients
+ (is (= {:ok true}
+ ((mt/user->client :rasta) :post 200 "pulse/test" (assoc result :channels [email-channel]))))
+ (is (= (mt/email-to :rasta {:subject "Pulse: A Pulse"
+ :body {"A Pulse" true}})
+ (mt/regex-email-bodies #"A Pulse"))))))))))
+
+(deftest dont-run-cards-async-test
+ (testing "A Card saved with `:async?` true should not be ran async for a Pulse"
+ (is (map? (#'pulse-api/pulse-card-query-results
+ {:id 1
+ :dataset_query {:database (mt/id)
+ :type :query
+ :query {:source-table (mt/id :venues)
+ :limit 1}
+ :async? true}})))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GET /api/pulse/form_input |
;;; +----------------------------------------------------------------------------------------------------------------+
-;; Check that Slack channels come back when configured
-(expect
- [{:name "channel", :type "select", :displayName "Post to", :options ["#foo" "@bar"], :required true}]
- (tu/with-temporary-setting-values [slack-token "something"]
- (with-redefs [slack/channels-list (constantly [{:name "foo"}])
- slack/users-list (constantly [{:name "bar"}])]
- (-> ((user->client :rasta) :get 200 "pulse/form_input")
- (get-in [:channels :slack :fields])))))
-
-;; When slack is not configured, `form_input` returns just the #genreal slack channel
-(expect
- [{:name "channel", :type "select", :displayName "Post to", :options ["#general"], :required true}]
- (tu/with-temporary-setting-values [slack-token nil]
- (-> ((user->client :rasta) :get 200 "pulse/form_input")
- (get-in [:channels :slack :fields]))))
+(deftest form-input-test
+ (testing "GET /api/pulse/form_input"
+ (testing "Check that Slack channels come back when configured"
+ (mt/with-temporary-setting-values [slack-token "something"]
+ (with-redefs [slack/channels-list (constantly [{:name "foo"}])
+ slack/users-list (constantly [{:name "bar"}])]
+ (is (= [{:name "channel", :type "select", :displayName "Post to", :options ["#foo" "@bar"], :required true}]
+ (-> ((mt/user->client :rasta) :get 200 "pulse/form_input")
+ (get-in [:channels :slack :fields])))))))
+
+ (testing "When slack is not configured, `form_input` returns just the #genreal slack channel"
+ (mt/with-temporary-setting-values [slack-token nil]
+ (is (= [{:name "channel", :type "select", :displayName "Post to", :options ["#general"], :required true}]
+ (-> ((mt/user->client :rasta) :get 200 "pulse/form_input")
+ (get-in [:channels :slack :fields]))))))))
+
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | GET /api/pulse/preview_card/:id |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(deftest preview-pulse-test
+ (testing "GET /api/pulse/preview_card/:id"
+ (mt/with-temp* [Collection [collection]
+ Card [card {:dataset_query (mt/mbql-query checkins {:limit 5})}]]
+ (letfn [(preview [expected-status-code]
+ (http/client-full-response (mt/user->credentials :rasta)
+ :get expected-status-code (format "pulse/preview_card_png/%d" (u/get-id card))))]
+ (testing "Should be able to preview a Pulse"
+ (let [{{:strs [Content-Type]} :headers, :keys [body]} (preview 200)]
+ (is (= "image/png"
+ Content-Type))
+ (is (some? body))))
+
+ (testing "If rendering a Pulse fails (e.g. because font registration failed) the endpoint should return the error message"
+ (with-redefs [png/register-fonts-if-needed! (fn []
+ (throw (ex-info "Can't register fonts!"
+ {}
+ (NullPointerException.))))]
+ (let [{{:strs [Content-Type]} :headers, :keys [body]} (preview 500)]
+ (is (= "application/json;charset=utf-8"
+ Content-Type))
+ (is (schema= {:message (s/eq "Can't register fonts!")
+ :trace s/Any
+ :via s/Any
+ s/Keyword s/Any}
+ body)))))))))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | DELETE /api/pulse/:pulse-id/subscription |
;;; +----------------------------------------------------------------------------------------------------------------+
-(expect
- nil
- (tt/with-temp* [Pulse [{pulse-id :id} {:name "Lodi Dodi" :creator_id (user->id :crowberto)}]
- PulseChannel [{channel-id :id} {:pulse_id pulse-id
- :channel_type "email"
- :schedule_type "daily"
- :details {:other "stuff"
- :emails ["foo@bar.com"]}}]
- PulseChannelRecipient [pcr {:pulse_channel_id channel-id :user_id (user->id :rasta)}]]
- ((user->client :rasta) :delete 204 (str "pulse/" pulse-id "/subscription/email"))))
-
-;; Users can't delete someone else's pulse subscription
-(expect
- "Not found."
- (tt/with-temp* [Pulse [{pulse-id :id} {:name "Lodi Dodi" :creator_id (user->id :crowberto)}]
- PulseChannel [{channel-id :id} {:pulse_id pulse-id
- :channel_type "email"
- :schedule_type "daily"
- :details {:other "stuff"
- :emails ["foo@bar.com"]}}]
- PulseChannelRecipient [pcr {:pulse_channel_id channel-id :user_id (user->id :rasta)}]]
- ((user->client :lucky) :delete 404 (str "pulse/" pulse-id "/subscription/email"))))
+(deftest delete-subscription-test
+ (testing "DELETE /api/pulse/:pulse-id/subscription"
+ (mt/with-temp* [Pulse [{pulse-id :id} {:name "Lodi Dodi" :creator_id (mt/user->id :crowberto)}]
+ PulseChannel [{channel-id :id} {:pulse_id pulse-id
+ :channel_type "email"
+ :schedule_type "daily"
+ :details {:other "stuff"
+ :emails ["foo@bar.com"]}}]]
+ (testing "Should be able to delete your own subscription"
+ (mt/with-temp PulseChannelRecipient [pcr {:pulse_channel_id channel-id :user_id (mt/user->id :rasta)}]
+ (is (= nil
+ ((mt/user->client :rasta) :delete 204 (str "pulse/" pulse-id "/subscription/email"))))))
+
+ (testing "Users can't delete someone else's pulse subscription"
+ (mt/with-temp PulseChannelRecipient [pcr {:pulse_channel_id channel-id :user_id (mt/user->id :rasta)}]
+ (is (= "Not found."
+ ((mt/user->client :lucky) :delete 404 (str "pulse/" pulse-id "/subscription/email")))))))))
diff --git a/test/metabase/async/api_response_test.clj b/test/metabase/async/api_response_test.clj
index ac4f6fb1e8a14c2e420f8a2a586edef1a6134d5a..3f84bb5e59b2562727db34beae159c780ace3f06 100644
--- a/test/metabase/async/api_response_test.clj
+++ b/test/metabase/async/api_response_test.clj
@@ -1,10 +1,12 @@
(ns metabase.async.api-response-test
(:require [cheshire.core :as json]
[clojure.core.async :as a]
+ [clojure.test :refer :all]
[expectations :refer [expect]]
[metabase.async.api-response :as async-response]
[metabase.test.util.async :as tu.async]
- [ring.core.protocols :as ring.protocols])
+ [ring.core.protocols :as ring.protocols]
+ [schema.core :as s])
(:import [java.io ByteArrayOutputStream Closeable]))
(def ^:private long-timeout-ms
@@ -48,13 +50,7 @@
true)
(defn- os->response [^ByteArrayOutputStream os]
- (some->
- os
- .toString
- (json/parse-string keyword)
- ((fn [response]
- (cond-> response
- (:stacktrace response) (update :stacktrace (partial every? string?)))))))
+ (some-> os .toString (json/parse-string keyword)))
;;; ------------------------------ Normal responses: message sent to the input channel -------------------------------
@@ -98,43 +94,37 @@
;;; ----------------------------------------- Input-chan closed unexpectedly -----------------------------------------
-;; if we close input-channel prematurely, output-channel should get closed
-(expect
- (tu.async/with-chans [input-chan]
- (with-response [{:keys [output-chan]} input-chan]
- ;; output-chan may or may not get the InterruptedException written to it -- it's a race condition -- we're just
- ;; want to make sure it closes
- (a/close! input-chan)
- (not= ::tu.async/timed-out (tu.async/wait-for-result output-chan)))))
-
-;; ...as should the output stream
-(expect
- (tu.async/with-chans [input-chan]
- (with-response [{:keys [os-closed-chan]} input-chan]
- (a/close! input-chan)
- (wait-for-close os-closed-chan))))
-
-;; An error should be written to the output stream
-(expect
- {:message "Input channel unexpectedly closed."
- :type "class java.lang.InterruptedException"
- :_status 500
- :stacktrace true}
- (tu.async/with-chans [input-chan]
- (with-response [{:keys [os os-closed-chan]} input-chan]
- (a/close! input-chan)
- (wait-for-close os-closed-chan)
- (os->response os))))
+(deftest input-chan-closed-unexpectedly-test
+ (testing "When input-channel is closed prematurely"
+ (tu.async/with-chans [input-chan]
+ (with-response [{:keys [os output-chan os-closed-chan]} input-chan]
+ (a/close! input-chan)
+ (testing "output-channel should get closed"
+ ;; output-chan may or may not get the InterruptedException written to it -- it's a race condition -- we're just
+ ;; want to make sure it closes
+ (not= ::tu.async/timed-out (tu.async/wait-for-result output-chan))
+
+ (testing "...as should the output stream"
+ (is (= true
+ (wait-for-close os-closed-chan)))))
+
+ (testing "An error should be written to the output stream"
+ (is (schema= {:message (s/eq "Input channel unexpectedly closed.")
+ :type (s/eq "class java.lang.InterruptedException")
+ :_status (s/eq 500)
+ :trace s/Any
+ s/Any s/Any}
+ (os->response os))))))))
;;; ------------------------------ Output-chan closed early (i.e. API request canceled) ------------------------------
;; If output-channel gets closed (presumably because the API request is canceled), input-chan should also get closed
(expect
- (tu.async/with-chans [input-chan]
- (with-response [{:keys [output-chan]} input-chan]
- (a/close! output-chan)
- (wait-for-close input-chan))))
+ (tu.async/with-chans [input-chan]
+ (with-response [{:keys [output-chan]} input-chan]
+ (a/close! output-chan)
+ (wait-for-close input-chan))))
;; if output chan gets closed, output-stream should also get closed
(expect
@@ -156,43 +146,42 @@
;;; --------------------------------------- input chan message is an Exception ---------------------------------------
-;; If the message sent to input-chan is an Exception an appropriate response should be generated
-(expect
- {:message "Broken", :type "class java.lang.Exception", :stacktrace true, :_status 500}
- (tu.async/with-chans [input-chan]
- (with-response [{:keys [os os-closed-chan]} input-chan]
- (a/>!! input-chan (Exception. "Broken"))
- (wait-for-close os-closed-chan)
- (os->response os))))
-
-
-;;; ------------------------------------------ input-chan never written to -------------------------------------------
-
-;; If we write a bad API endpoint and return a channel but never write to it, the request should be canceled after
-;; `absolute-max-keepalive-ms`
-(expect
- {:message "No response after waiting 500.0 ms. Canceling request."
- :type "class java.util.concurrent.TimeoutException"
- :_status 500
- :stacktrace true}
- (with-redefs [async-response/absolute-max-keepalive-ms 500]
+(deftest input-change-message-is-exception-test
+ (testing "If the message sent to input-chan is an Exception an appropriate response should be generated"
(tu.async/with-chans [input-chan]
(with-response [{:keys [os os-closed-chan]} input-chan]
+ (a/>!! input-chan (Exception. "Broken"))
(wait-for-close os-closed-chan)
- (os->response os)))))
+ (is (schema= {:message (s/eq "Broken")
+ :type (s/eq "class java.lang.Exception")
+ :trace s/Any
+ :_status (s/eq 500)
+ s/Keyword s/Any}
+ (os->response os)))))))
-;; input chan should get closed
-(expect
- (with-redefs [async-response/absolute-max-keepalive-ms 500]
- (tu.async/with-chans [input-chan]
- (with-response [{:keys [os-closed-chan]} input-chan]
- (wait-for-close os-closed-chan)
- (wait-for-close input-chan)))))
-;; output chan should get closed
-(expect
- (with-redefs [async-response/absolute-max-keepalive-ms 500]
- (tu.async/with-chans [input-chan]
- (with-response [{:keys [output-chan os-closed-chan]} input-chan]
- (wait-for-close os-closed-chan)
- (wait-for-close output-chan)))))
+;;; ------------------------------------------ input-chan never written to -------------------------------------------
+
+(deftest input-chan-never-written-to-test
+ (testing (str "If we write a bad API endpoint and return a channel but never write to it, the request should be "
+ "canceled after `absolute-max-keepalive-ms`")
+ (with-redefs [async-response/absolute-max-keepalive-ms 50]
+ (tu.async/with-chans [input-chan]
+ (with-response [{:keys [os os-closed-chan output-chan]} input-chan]
+ (is (= true
+ (wait-for-close os-closed-chan)))
+ (testing "error should be written to output stream"
+ (is (schema= {:message (s/eq "No response after waiting 50.0 ms. Canceling request.")
+ :type (s/eq "class java.util.concurrent.TimeoutException")
+ :_status (s/eq 500)
+ :trace s/Any
+ s/Keyword s/Any}
+ (os->response os))))
+
+ (testing "input chan should get closed"
+ (is (= true
+ (wait-for-close input-chan))))
+
+ (testing "output chan should get closed"
+ (is (= true
+ (wait-for-close output-chan)))))))))
diff --git a/test/metabase/pulse/render/png_test.clj b/test/metabase/pulse/render/png_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..34aa760d20915619876869e56be58a3e14cffc83
--- /dev/null
+++ b/test/metabase/pulse/render/png_test.clj
@@ -0,0 +1,24 @@
+(ns metabase.pulse.render.png-test
+ (:require [clojure.test :refer :all]
+ [metabase.pulse.render.png :as png]
+ [metabase.test :as mt]
+ [schema.core :as s]))
+
+(deftest register-fonts-test
+ (testing "Under normal circumstances, font registration should work as expected"
+ (is (= nil
+ (#'png/register-fonts-if-needed!))))
+
+ (testing "If font regsitration fails, we should an Exception with a useful error message"
+ (with-redefs [png/register-font! (fn [& _]
+ (throw (ex-info "Oops!" {})))]
+ (let [messages (mt/with-log-messages
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"Error registering fonts: Metabase will not be able to send Pulses"
+ (#'png/register-fonts!))))]
+ (testing "Should log the Exception"
+ (is (schema= [(s/one (s/eq :error) "log type")
+ (s/one Throwable "exception")
+ (s/one #"^Error registering fonts" "message")]
+ (first messages))))))))
diff --git a/test/metabase/test.clj b/test/metabase/test.clj
index 2b2bd8c73f979889dff6cc7865e5cf46c4cb725e..fcf734580c24e980b280fb3cdb3ac8eccaacf636 100644
--- a/test/metabase/test.clj
+++ b/test/metabase/test.clj
@@ -8,6 +8,7 @@
[medley.core :as m]
[metabase
[driver :as driver]
+ [email-test :as et]
[query-processor :as qp]
[query-processor-test :as qp.test]]
[metabase.driver.sql-jdbc.test-util :as sql-jdbc.tu]
@@ -37,12 +38,13 @@
data/keep-me
datasets/keep-me
driver/keep-me
+ et/keep-me
initialize/keep-me
qp/keep-me
qp.test-util/keep-me
qp.test/keep-me
sql-jdbc.tu/keep-me
- [test-users/keep-me]
+ test-users/keep-me
tt/keep-me
tu/keep-me
tu.async/keep-me
@@ -76,6 +78,16 @@
*driver*
with-driver]
+ [et
+ email-to
+ fake-inbox-email-fn
+ inbox
+ regex-email-bodies
+ reset-inbox!
+ summarize-multipart-email
+ with-expected-messages
+ with-fake-inbox]
+
[initialize
initialize-if-needed!]
@@ -107,6 +119,7 @@
sql-jdbc-drivers]
[test-users
+ fetch-user
user->id
user->client
user->credentials
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index 81a7810f6aa5d32927bb6e6dd64289d3197e7e86..f780a833a61c57113b9d41bdb0070b127c3a4f56 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -361,7 +361,7 @@
@messages))
(defmacro with-log-messages
- "Execute BODY, and return a vector of all messages logged using the `log/` family of functions. Messages are of the
+ "Execute `body`, and return a vector of all messages logged using the `log/` family of functions. Messages are of the
format `[:level throwable message]`, and are returned in chronological order from oldest to newest.
(with-log-messages (log/warn \"WOW\")) ; -> [[:warn nil \"WOW\"]]"