From ffd0b0eafe24cbfdb23643673d0632922337e367 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Thu, 14 May 2020 19:13:16 -0700
Subject: [PATCH] Fix DockerHub build & better Pulse render error logging
(#12543)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Change font registration to run on first Pulse PNG render rather than at launch.
* This will improve startup times slightly 🚀.
* This was causing DockerHub builds for `metabase-head` to fail
because the stage 1 "builder" image did not have `ttf-dejavu` and `fontconfig` installed (the builder was running in to #12223).
* This will resolve issues discussed in #12223 where Metabase fails to launch when running on a "headless" JVM Docker image (i.e., one without AWT libraries) or when we run into the infamous font issues. We still won't be able to render Pulses, but Metabase without Pulses is slightly preferable to no Metabase at all
* Improved error messages & logging when Pulse rendering fails. Unfortunately, if the AWT classes are unavailable/broken in the JVM there is no way we can render Pulses given our current reliance on it. The best we can do is explain the situation and point people in the right direction. The new error message explains the situation and points people to #7986 for more details and workarounds. As such, I am going to close #7986 because we cannot fix the underlying issue itself so the best we do is handle it better.
* Refactor tests in `metabase.api.pulse-test`
* Add tests for `GET /api/pulse/preview_card/:id`
* Include entire Exception chain in 500 responses
* Fix font registration logic not cleaning up `InputStream`s when done with them, wasting memory
* Minor Dockerfile tweaks
---
Dockerfile | 6 +-
bin/docker/Dockerfile | 2 +-
src/metabase/api/pulse.clj | 3 +-
src/metabase/middleware/exceptions.clj | 13 +-
src/metabase/pulse/render/png.clj | 62 +-
test/metabase/api/card_test.clj | 523 ++++----
test/metabase/api/pulse_test.clj | 1473 ++++++++++-----------
test/metabase/async/api_response_test.clj | 137 +-
test/metabase/pulse/render/png_test.clj | 24 +
test/metabase/test.clj | 15 +-
test/metabase/test/util.clj | 2 +-
11 files changed, 1119 insertions(+), 1141 deletions(-)
create mode 100644 test/metabase/pulse/render/png_test.clj
diff --git a/Dockerfile b/Dockerfile
index 854c3b0eda8..1793f9eac95 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 e212a778ec9..bdc1cb0a9fe 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 26a4d0d1c82..cb353128bd5 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 06be9b90315..d36e8522ffb 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 1d1da38a5b0..df904d33831 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 07632c77311..eb10cd34e9d 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 8a81833c5ca..c3293fd05e5 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 ac4f6fb1e8a..3f84bb5e59b 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 00000000000..34aa760d209
--- /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 b63997f2c47..ce85e68226f 100644
--- a/test/metabase/test.clj
+++ b/test/metabase/test.clj
@@ -10,6 +10,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]
@@ -39,12 +40,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
@@ -78,6 +80,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!]
@@ -109,6 +121,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 81a7810f6aa..f780a833a61 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\"]]"
--
GitLab