diff --git a/src/metabase/pulse.clj b/src/metabase/pulse.clj
index f79703af77347250861d9628b6f2492b2fda7fa7..2197905bab65a1a3d58cbf11f4bc96fbfc8ce1c9 100644
--- a/src/metabase/pulse.clj
+++ b/src/metabase/pulse.clj
@@ -115,16 +115,16 @@
:below (trs "gone below its goal")
:rows (trs "results")))
-(def ^:private ^:dynamic *slack-mrkdwn-length-limit*
- 3000)
+(def ^:private block-text-length-limit 3000)
+(def ^:private attachment-text-length-limit 2000)
(defn- truncate-mrkdwn
"If a mrkdwn string is greater than Slack's length limit, truncates it to fit the limit and
adds an ellipsis character to the end."
- [mrkdwn]
- (if (> (count mrkdwn) *slack-mrkdwn-length-limit*)
+ [mrkdwn limit]
+ (if (> (count mrkdwn) limit)
(-> mrkdwn
- (subs 0 (dec *slack-mrkdwn-length-limit*))
+ (subs 0 (dec limit))
(str "…"))
mrkdwn))
@@ -146,7 +146,7 @@
(when (not (str/blank? mrkdwn))
{:blocks [{:type "section"
:text {:type "mrkdwn"
- :text (truncate-mrkdwn mrkdwn)}}]})))))
+ :text (truncate-mrkdwn mrkdwn block-text-length-limit)}}]})))))
(remove nil?))))
(defn- subject
@@ -156,6 +156,12 @@
name
(trs "Pulse: {0}" name)))
+(defn- filter-text
+ [filter]
+ (truncate-mrkdwn
+ (format "*%s*\n%s" (:name filter) (params/value-string filter))
+ attachment-text-length-limit))
+
(defn- slack-dashboard-header
"Returns a block element that includes a dashboard's name, creator, and filters, for inclusion in a
Slack dashboard subscription"
@@ -170,7 +176,7 @@
filters (params/parameters pulse dashboard)
filter-fields (for [filter filters]
{:type "mrkdwn"
- :text (str "*" (:name filter) "*\n" (params/value-string filter))})
+ :text (filter-text filter)})
filter-section (when (seq filter-fields)
{:type "section"
:fields filter-fields})]
diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj
index 84c50d54e6df4f7305cda56bc5ec27cd79413179..52f17af90383ee05b00b22d74ec53777dd0e0c5d 100644
--- a/test/metabase/dashboard_subscription_test.clj
+++ b/test/metabase/dashboard_subscription_test.clj
@@ -289,70 +289,71 @@
(pulse.test-util/thunk->boolean pulse-results)))))}}))
(deftest dashboard-filter-test
- (tests {:pulse {:skip_if_empty false}
- :dashboard pulse.test-util/test-dashboard}
- "Dashboard subscription that includes a dashboard filters"
- {:card (pulse.test-util/checkins-query-card {})
-
- :fixture
- (fn [_ thunk]
- (mt/with-temporary-setting-values [site-name "Metabase Test"]
- (thunk)))
-
- :assert
- {:email
- (fn [_ _]
- (testing "Markdown cards are included in email subscriptions"
- (is (= (rasta-pulse-email {:body [{"Aviary KPIs" true
- "<a class=\\\"title\\\" href=\\\"https://metabase.com/testmb/dashboard/\\d+\\?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021\\\"" true}
- pulse.test-util/png-attachment]})
- (mt/summarize-multipart-email #"Aviary KPIs"
- #"<a class=\"title\" href=\"https://metabase.com/testmb/dashboard/\d+\?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021\"")))))
-
- :slack
- (fn [{:keys [card-id dashboard-id]} [pulse-results]]
- (testing "Markdown cards are included in attachments list as :blocks sublists, and markdown is
- converted to mrkdwn (Slack markup language)"
- (is (= {:channel-id "#general"
- :attachments
- [{:blocks [{:type "header", :text {:type "plain_text", :text "Aviary KPIs", :emoji true}}
- {:type "section",
- :fields [{:type "mrkdwn", :text "*State*\nCA, NY and NJ"}
- {:type "mrkdwn", :text "*Quarter and Year*\nQ1, 2021"}]}
- {:type "section", :fields [{:type "mrkdwn", :text "Sent by Rasta Toucan"}]}]}
- {:title pulse.test-util/card-name
- :rendered-info {:attachments false, :content true, :render/text true},
- :title_link (str "https://metabase.com/testmb/question/" card-id)
- :attachment-name "image.png"
- :channel-id "FOO"
- :fallback pulse.test-util/card-name}
- {:blocks [{:type "divider"}
- {:type "context"
- :elements [{:type "mrkdwn"
- :text (str "<https://metabase.com/testmb/dashboard/"
- dashboard-id
- "?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021|*Sent from Metabase Test*>")}]}]}]}
- (pulse.test-util/thunk->boolean pulse-results)))))}}))
+ (with-redefs [metabase.pulse/attachment-text-length-limit 15]
+ (tests {:pulse {:skip_if_empty false}
+ :dashboard pulse.test-util/test-dashboard}
+ "Dashboard subscription that includes a dashboard filters"
+ {:card (pulse.test-util/checkins-query-card {})
+
+ :fixture
+ (fn [_ thunk]
+ (mt/with-temporary-setting-values [site-name "Metabase Test"]
+ (thunk)))
+
+ :assert
+ {:email
+ (fn [_ _]
+ (testing "Markdown cards are included in email subscriptions"
+ (is (= (rasta-pulse-email {:body [{"Aviary KPIs" true
+ "<a class=\\\"title\\\" href=\\\"https://metabase.com/testmb/dashboard/\\d+\\?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021\\\"" true}
+ pulse.test-util/png-attachment]})
+ (mt/summarize-multipart-email #"Aviary KPIs"
+ #"<a class=\"title\" href=\"https://metabase.com/testmb/dashboard/\d+\?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021\"")))))
+
+ :slack
+ (fn [{:keys [card-id dashboard-id]} [pulse-results]]
+ (testing "Markdown cards are included in attachments list as :blocks sublists, and markdown is
+ converted to mrkdwn (Slack markup language) and truncated appropriately"
+ (is (= {:channel-id "#general"
+ :attachments
+ [{:blocks [{:type "header", :text {:type "plain_text", :text "Aviary KPIs", :emoji true}}
+ {:type "section",
+ :fields [{:type "mrkdwn", :text "*State*\nCA, NY…"} ;; "*State*\nCA, NY and NJ"
+ {:type "mrkdwn", :text "*Quarter and Y…"}]} ;; "*Quarter and Year*\nQ1, 2021"
+ {:type "section", :fields [{:type "mrkdwn", :text "Sent by Rasta Toucan"}]}]}
+ {:title pulse.test-util/card-name
+ :rendered-info {:attachments false, :content true, :render/text true},
+ :title_link (str "https://metabase.com/testmb/question/" card-id)
+ :attachment-name "image.png"
+ :channel-id "FOO"
+ :fallback pulse.test-util/card-name}
+ {:blocks [{:type "divider"}
+ {:type "context"
+ :elements [{:type "mrkdwn"
+ :text (str "<https://metabase.com/testmb/dashboard/"
+ dashboard-id
+ "?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021|*Sent from Metabase Test*>")}]}]}]}
+ (pulse.test-util/thunk->boolean pulse-results)))))}})))
(deftest mrkdwn-length-limit-test
- (tests {:pulse {:skip_if_empty false}, :dashcard {:row 0, :col 0}}
- "Dashboard subscription that includes a Markdown card that exceeds Slack's length limit when converted to mrkdwn"
- {:card (pulse.test-util/checkins-query-card {})
-
- :fixture
- (fn [{dashboard-id :dashboard-id} thunk]
- (mt/with-temp DashboardCard [_ {:dashboard_id dashboard-id
- :row 1
- :col 1
- :visualization_settings {:text "abcdefghijklmnopqrstuvwxyz"}}]
- (binding [metabase.pulse/*slack-mrkdwn-length-limit* 10]
- (thunk))))
-
- :assert
- {:slack
- (fn [_object-ids [pulse-results]]
- (is (= {:blocks [{:type "section" :text {:type "mrkdwn" :text "abcdefghi…"}}]}
- (nth (:attachments (pulse.test-util/thunk->boolean pulse-results)) 2))))}}))
+ (with-redefs [metabase.pulse/block-text-length-limit 10]
+ (tests {:pulse {:skip_if_empty false}, :dashcard {:row 0, :col 0}}
+ "Dashboard subscription that includes a Markdown card that exceeds Slack's length limit when converted to mrkdwn"
+ {:card (pulse.test-util/checkins-query-card {})
+
+ :fixture
+ (fn [{dashboard-id :dashboard-id} thunk]
+ (mt/with-temp DashboardCard [_ {:dashboard_id dashboard-id
+ :row 1
+ :col 1
+ :visualization_settings {:text "abcdefghijklmnopqrstuvwxyz"}}]
+ (thunk)))
+
+ :assert
+ {:slack
+ (fn [_object-ids [pulse-results]]
+ (is (= {:blocks [{:type "section" :text {:type "mrkdwn" :text "abcdefghi…"}}]}
+ (nth (:attachments (pulse.test-util/thunk->boolean pulse-results)) 2))))}})))
(deftest archived-dashboard-test
(tests {:dashboard {:archived true}}