From b30e2e535145efa4b4e6db74750dbeaf1e30083b Mon Sep 17 00:00:00 2001
From: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Date: Wed, 20 Jul 2022 13:37:47 -0400
Subject: [PATCH] Include markdown parameters in dashboard subscriptions
(#24070)
* basic logic for including params in dashboard subscriptions
* BE date formatting
* redo approach to date formatting locale
* refactor and use the same formatting code for parameters listed in subscription header
* fix tests and lint
* fix more tests
* fix final (?) test and fix namespace lint error
* a couple extra test cases
* fix tests
* new cypress test
* address braden and ngoc's comments
* fix tests
* guard against nil text in substitute_tags
---
frontend/src/metabase/lib/i18n.js | 23 +++-
.../visualizations/Text/Text.jsx | 6 +-
.../dashboard/text-parameters.cy.spec.js | 63 +++++++++++
.../shared/parameters/parameters.cljc | 105 +++++++++++-------
.../shared/parameters/parameters_test.cljc | 67 +++++++----
src/metabase/pulse.clj | 30 ++++-
src/metabase/pulse/parameters.clj | 11 +-
test/metabase/dashboard_subscription_test.clj | 25 ++++-
test/metabase/pulse/parameters_test.clj | 10 +-
test/metabase/pulse/test_util.clj | 2 +-
10 files changed, 252 insertions(+), 90 deletions(-)
diff --git a/frontend/src/metabase/lib/i18n.js b/frontend/src/metabase/lib/i18n.js
index ea52fd4966f..1251237c68a 100644
--- a/frontend/src/metabase/lib/i18n.js
+++ b/frontend/src/metabase/lib/i18n.js
@@ -58,15 +58,20 @@ function updateMomentStartOfWeek() {
// if the start of week Setting is updated, update the moment start of week
MetabaseSettings.on("start-of-week", updateMomentStartOfWeek);
-export function setLocalization(translationsObject) {
+function setLanguage(translationsObject) {
const locale = translationsObject.headers.language;
-
addMsgIds(translationsObject);
- // add and set locale with C-3PO
+ // add and set locale with ttag
addLocale(locale, translationsObject);
// eslint-disable-next-line react-hooks/rules-of-hooks
useLocale(locale);
+}
+
+function setLocalization(translationsObject) {
+ const locale = translationsObject.headers.language;
+
+ setLanguage(translationsObject);
updateMomentLocale(locale);
updateMomentStartOfWeek(locale);
@@ -116,19 +121,25 @@ function addMsgIds(translationsObject) {
// Runs `f` with the current language for ttag set to the instance (site) locale rather than the user locale, then
// restores the user locale. This can be used for translating specific strings into the instance language; e.g. for
// parameter values in dashboard text cards that should be translated the same for all users viewing the dashboard.
-export function withInstanceLocalization(f) {
+export function withInstanceLanguage(f) {
if (window.MetabaseSiteLocalization) {
- setLocalization(window.MetabaseSiteLocalization);
+ setLanguage(window.MetabaseSiteLocalization);
}
try {
return f();
} finally {
if (window.MetabaseUserLocalization) {
- setLocalization(window.MetabaseUserLocalization);
+ setLanguage(window.MetabaseUserLocalization);
}
}
}
+export function siteLocale() {
+ if (window.MetabaseSiteLocalization) {
+ return window.MetabaseSiteLocalization.headers.language;
+ }
+}
+
// register site locale with ttag, if needed later
if (window.MetabaseSiteLocalization) {
const translationsObject = window.MetabaseSiteLocalization;
diff --git a/frontend/src/metabase/visualizations/visualizations/Text/Text.jsx b/frontend/src/metabase/visualizations/visualizations/Text/Text.jsx
index 8f427787d15..010b2d21313 100644
--- a/frontend/src/metabase/visualizations/visualizations/Text/Text.jsx
+++ b/frontend/src/metabase/visualizations/visualizations/Text/Text.jsx
@@ -8,7 +8,7 @@ import _ from "underscore";
import cx from "classnames";
import { t } from "ttag";
-import { withInstanceLocalization } from "metabase/lib/i18n";
+import { withInstanceLanguage, siteLocale } from "metabase/lib/i18n";
import { substitute_tags } from "cljs/metabase.shared.parameters.parameters";
@@ -135,8 +135,8 @@ export default class Text extends Component {
if (!_.isEmpty(parametersByTag)) {
// Temporarily override language to use site language, so that all viewers of a dashboard see parameter values
// translated the same way.
- content = withInstanceLocalization(() =>
- substitute_tags(content, parametersByTag),
+ content = withInstanceLanguage(() =>
+ substitute_tags(content, parametersByTag, siteLocale()),
);
}
diff --git a/frontend/test/metabase/scenarios/dashboard/text-parameters.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/text-parameters.cy.spec.js
index 8d6bd064ff2..86d88f15164 100644
--- a/frontend/test/metabase/scenarios/dashboard/text-parameters.cy.spec.js
+++ b/frontend/test/metabase/scenarios/dashboard/text-parameters.cy.spec.js
@@ -8,6 +8,9 @@ import {
addTextBox,
popover,
} from "__support__/e2e/helpers";
+import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database";
+
+const { PRODUCTS_ID } = SAMPLE_DATABASE;
describe("scenarios > dashboard > parameters in text cards", () => {
beforeEach(() => {
@@ -87,4 +90,64 @@ describe("scenarios > dashboard > parameters in text cards", () => {
cy.visit("/");
cy.findByText("Pick up where you left off").should("exist");
});
+
+ it("should localize date parameters in the instance locale", () => {
+ cy.request("GET", "/api/user/current").then(({ body: { id: USER_ID } }) => {
+ cy.request("PUT", `/api/user/${USER_ID}`, { locale: "en" });
+ });
+ cy.request("PUT", `/api/setting/site-locale`, { value: "fr" });
+
+ // Create dashboard with a single date parameter, and a single question
+ cy.createQuestionAndDashboard({
+ questionDetails: { query: { "source-table": PRODUCTS_ID } },
+ }).then(({ body: card }) => {
+ const { dashboard_id } = card;
+ cy.request("PUT", `/api/dashboard/${dashboard_id}`, {
+ parameters: [
+ {
+ name: "Single Date",
+ slug: "single_date",
+ id: "ad1c877e",
+ type: "date/single",
+ },
+ ],
+ });
+ const updatedSize = {
+ sizeX: 8,
+ sizeY: 6,
+ };
+ cy.editDashboardCard(card, updatedSize);
+ visitDashboard(dashboard_id);
+
+ // Connect parameter to question
+ editDashboard();
+ cy.findByText("Single Date").click();
+ cy.findByText("Select…").click();
+ cy.findByText("Created At").click();
+ cy.findByText("Single Date").click();
+
+ // Create text card and connect parameter
+ addTextBox("Variable: {{foo}}", { parseSpecialCharSequences: false });
+ cy.findByText("Single Date").click();
+ cy.findByText("Select…").click();
+ cy.findByText("foo").click();
+ saveDashboard();
+
+ cy.findByText("Single Date").click();
+ popover().within(() => {
+ cy.findByRole("textbox").click().clear().type("07/19/2017");
+ cy.button("Update filter").click();
+ });
+
+ // Question should be filtered appropriately
+ cy.findByText("Rustic Paper Wallet").should("exist");
+ cy.findByText("Small Marble Shoes").should("not.exist");
+
+ // Parameter value in widget should use user localization (English)
+ cy.findByText("July 19, 2017").should("exist");
+
+ // Parameter value in dashboard should use site localization (French)
+ cy.findByText("Variable: juillet 19, 2017").should("exist");
+ });
+ });
});
diff --git a/shared/src/metabase/shared/parameters/parameters.cljc b/shared/src/metabase/shared/parameters/parameters.cljc
index 95b7fa50c5b..9f72615c235 100644
--- a/shared/src/metabase/shared/parameters/parameters.cljc
+++ b/shared/src/metabase/shared/parameters/parameters.cljc
@@ -4,51 +4,66 @@
(:clj
[(:require [clojure.string :as str]
[metabase.mbql.normalize :as mbql.normalize]
- [metabase.shared.util.i18n :refer [trs]])]
+ [metabase.shared.util.i18n :refer [trs]]
+ [metabase.util.date-2 :as u.date]
+ [metabase.util.date-2.parse.builder :as b]
+ [metabase.util.i18n.impl :as i18n.impl])
+ (:import java.time.format.DateTimeFormatter)]
:cljs
[(:require ["moment" :as moment]
[clojure.string :as str]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.shared.util.i18n :refer [trs]])]))
-(defn- formatted-list
- [values]
- (str (str/join ", " (butlast values)) " " (trs "and") " " (last values)))
+;; Without this comment, the namespace-checker linter incorrectly detects moment as unused
+#?(:cljs (comment moment/keep-me))
(defmulti formatted-value
"Formats a value appropriately for inclusion in a text card, based on its type. Does not do any escaping.
For datetime parameters, the logic here should mirror the logic (as best as possible) in
frontend/src/metabase/parameters/utils/date-formatting.ts"
- (fn [tyype _value] (keyword tyype)))
+ (fn [tyype _value _locale] (keyword tyype)))
(defmethod formatted-value :date/single
- [_ value]
- #?(:cljs (.format (moment value) "MMMM D, YYYY")
- :clj value))
+ [_ value locale]
+ #?(:cljs (let [m (.locale (moment value) locale)]
+ (.format m "MMMM D, YYYY"))
+ :clj (u.date/format "MMMM d, yyyy" (u.date/parse value) locale)))
(defmethod formatted-value :date/month-year
- [_ value]
- #?(:cljs (let [m (moment value "YYYY-MM")]
+ [_ value locale]
+ #?(:cljs (let [m (.locale (moment value "YYYY-MM") locale)]
(if (.isValid m) (.format m "MMMM, YYYY") ""))
- :clj value))
+ :clj (u.date/format "MMMM, yyyy" (u.date/parse value) locale)))
+
+#?(:clj
+ (def ^:private quarter-formatter-in
+ (b/formatter
+ "Q" (b/value :iso/quarter-of-year 1) "-" (b/value :year 4))))
+
+#?(:clj
+ (def ^:private quarter-formatter-out
+ (b/formatter
+ "Q" (b/value :iso/quarter-of-year 1) ", " (b/value :year 4))))
(defmethod formatted-value :date/quarter-year
- [_ value]
- #?(:cljs (let [m (moment value "[Q]Q-YYYY")]
+ [_ value locale]
+ #?(:cljs (let [m (.locale (moment value "[Q]Q-YYYY") locale)]
(if (.isValid m) (.format m "[Q]Q, YYYY") ""))
- :clj value))
+ :clj (.format (.withLocale ^DateTimeFormatter quarter-formatter-out (i18n.impl/locale locale))
+ (.parse ^DateTimeFormatter quarter-formatter-in value))))
(defmethod formatted-value :date/range
- [_ value]
- (let [[start end] (str/split value "~")]
+ [_ value locale]
+ (let [[start end] (str/split value #"~")]
(if (and start end)
- (str (formatted-value :date/single start)
+ (str (formatted-value :date/single start locale)
" - "
- (formatted-value :date/single end))
+ (formatted-value :date/single end locale))
"")))
(defmethod formatted-value :date/relative
- [_ value]
+ [_ value locale]
(case value
"today" (trs "Today")
"yesterday" (trs "Yesterday")
@@ -62,26 +77,31 @@
"thismonth" (trs "This Month")
"thisyear" (trs "This Year")
;; Always fallback to default formatting, just in case
- (formatted-value :default value)))
+ (formatted-value :default locale locale)))
(defmethod formatted-value :date/all-options
- [_ value]
+ [_ value locale]
;; Test value against a series of regexes (similar to those in metabase/parameters/utils/mbql.js) to determine
;; the appropriate formatting, since it is not encoded in the parameter type.
;; TODO: this is a partial implementation that only handles simple dates
(condp (fn [re value] (->> (re-find re value) second)) value
- #"^~?([0-9-T:]+)~?$" :>> (partial formatted-value :date/single)
- #"^([0-9-T:]+~[0-9-T:]+)$" :>> (partial formatted-value :date/range)
+ #"^~?([0-9-T:]+)~?$" :>> #(formatted-value :date/single % locale)
+ #"^([0-9-T:]+~[0-9-T:]+)$" :>> #(formatted-value :date/range % locale)
(str value)))
+(defn formatted-list
+ "Given a seq of parameter values, returns them as a single comma-separated string. Does not do additional formatting
+ on the values."
+ [values]
+ (if (= (count values) 1)
+ (str (first values))
+ (str (str/join ", " (butlast values)) (trs " and ") (last values))))
+
(defmethod formatted-value :default
- [_ value]
+ [_ value _]
(cond
- (and (sequential? value) (> (count value) 1))
- (formatted-list value)
-
(sequential? value)
- (str (first value))
+ (formatted-list value)
:else
(str value)))
@@ -94,14 +114,18 @@
(str/replace text escaped-chars-regex #(str \\ %)))
(defn- replacement
- [tag->param match]
+ [tag->param locale match]
(let [tag-name (second match)
param (get tag->param tag-name)
value (:value param)
tyype (:type param)]
(if value
- (-> (formatted-value tyype value)
- escape-chars)
+ (try (-> (formatted-value tyype value locale)
+ escape-chars)
+ (catch #?(:clj Throwable :cljs js/Error) _
+ ;; If we got an exception (most likely during date parsing/formatting), fallback to the default
+ ;; implementation of formatted-value
+ (formatted-value :default value locale)))
;; If this parameter has no value, return the original {{tag}} so that no substitution is done.
(first match))))
@@ -131,11 +155,14 @@
(defn ^:export substitute_tags
"Given the context of a text dashboard card, replace all template tags in the text with their corresponding values,
formatted and escaped appropriately."
- [text tag->param]
- (let [tag->param #?(:clj tag->param
- :cljs (js->clj tag->param))
- tag->normalized-param (reduce-kv (fn [acc tag param]
- (assoc acc tag (normalize-parameter param)))
- {}
- tag->param)]
- (str/replace text template-tag-regex (partial replacement tag->normalized-param))))
+ ([text tag->param]
+ (substitute_tags text tag->param "en"))
+ ([text tag->param locale]
+ (when text
+ (let [tag->param #?(:clj tag->param
+ :cljs (js->clj tag->param))
+ tag->normalized-param (reduce-kv (fn [acc tag param]
+ (assoc acc tag (normalize-parameter param)))
+ {}
+ tag->param)]
+ (str/replace text template-tag-regex (partial replacement tag->normalized-param locale))))))
diff --git a/shared/test/metabase/shared/parameters/parameters_test.cljc b/shared/test/metabase/shared/parameters/parameters_test.cljc
index d7b95f2b628..921199971ae 100644
--- a/shared/test/metabase/shared/parameters/parameters_test.cljc
+++ b/shared/test/metabase/shared/parameters/parameters_test.cljc
@@ -116,6 +116,11 @@
(t/testing "No substitution is done when no parameter is provided, or the parameter is invalid"
(t/are [text tag->param expected] (= expected (params/substitute_tags text tag->param))
+ ;; Nil input
+ nil
+ {}
+ nil
+
;; No parameters
"{{foo}}"
{}
@@ -141,26 +146,42 @@
{"foo" {:value "today"}}
"today"))
- #?(:cljs
- (t/testing "Date/time values are formatted correctly when called in CLJS (TODO: update this test when Clojure
- implementation is added)"
- (t/are [text tag->param expected] (= expected (params/substitute_tags text tag->param))
- "{{foo}}"
- {"foo" {:type :date/single :value "2022-07-09"}}
- "July 9\\, 2022"
-
- "{{foo}}"
- {"foo" {:type :date/range :value "2022-07-06~2022-07-09"}}
- "July 6\\, 2022 \\- July 9\\, 2022"
-
- "{{foo}}"
- {"foo" {:type :date/month-year :value "2022-07"}}
- "July\\, 2022"
-
- "{{foo}}"
- {"foo" {:type :date/all-options :value "~2022-07-09"}}
- "July 9\\, 2022"
-
- "{{foo}}"
- {"foo" {:type :date/all-options :value "2022-07-06~2022-07-09"}}
- "July 6\\, 2022 \\- July 9\\, 2022"))))
+ (t/testing "Date values are formatted correctly"
+ (t/are [text tag->param expected] (= expected (params/substitute_tags text tag->param))
+ "{{foo}}"
+ {"foo" {:type :date/single :value "2022-07-09"}}
+ "July 9\\, 2022"
+
+ "{{foo}}"
+ {"foo" {:type :date/range :value "2022-07-06~2022-07-09"}}
+ "July 6\\, 2022 \\- July 9\\, 2022"
+
+ "{{foo}}"
+ {"foo" {:type :date/month-year :value "2022-07"}}
+ "July\\, 2022"
+
+ "{{foo}}"
+ {"foo" {:type :date/quarter-year :value "Q2-2022"}}
+ "Q2\\, 2022"
+
+ "{{foo}}"
+ {"foo" {:type :date/all-options :value "~2022-07-09"}}
+ "July 9\\, 2022"
+
+ "{{foo}}"
+ {"foo" {:type :date/all-options :value "2022-07-06~2022-07-09"}}
+ "July 6\\, 2022 \\- July 9\\, 2022")
+
+ (t/testing "Date values are formatted using the locale passed in as an argument"
+ (t/are [text tag->param expected] (= expected (params/substitute_tags text tag->param "es"))
+ "{{foo}}"
+ {"foo" {:type :date/single :value "2022-07-09"}}
+ "julio 9\\, 2022"
+
+ "{{foo}}"
+ {"foo" {:type :date/range :value "2022-01-06~2022-04-09"}}
+ "enero 6\\, 2022 \\- abril 9\\, 2022"
+
+ "{{foo}}"
+ {"foo" {:type :date/month-year :value "2019-08"}}
+ "agosto\\, 2019"))))
diff --git a/src/metabase/pulse.clj b/src/metabase/pulse.clj
index 3cfd6e70ece..9d9600728a9 100644
--- a/src/metabase/pulse.clj
+++ b/src/metabase/pulse.clj
@@ -21,6 +21,7 @@
[metabase.query-processor.dashboard :as qp.dashboard]
[metabase.query-processor.timezone :as qp.timezone]
[metabase.server.middleware.session :as mw.session]
+ [metabase.shared.parameters.parameters :as shared.params]
[metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.retry :as retry]
@@ -45,6 +46,22 @@
{:value default-value})
(dissoc parameter :default))))
+(defn- process-virtual-dashcard
+ "Given a dashcard and the parameters on a dashboard, returns the dashcard with any parameter values appropriately
+ substituted into connected variables in the text."
+ [dashcard parameters]
+ (let [text (-> dashcard :visualization_settings :text)
+ parameter-mappings (:parameter_mappings dashcard)
+ tag-names (shared.params/tag_names text)
+ param-id->param (into {} (map (juxt :id identity) parameters))
+ tag-name->param-id (into {} (map (juxt (comp second :target) :parameter_id) parameter-mappings))
+ tag->param (reduce (fn [m tag-name]
+ (when-let [param-id (get tag-name->param-id tag-name)]
+ (assoc m tag-name (get param-id->param param-id))))
+ {}
+ tag-names)]
+ (update-in dashcard [:visualization_settings :text] shared.params/substitute_tags tag->param (public-settings/site-locale))))
+
(defn- execute-dashboard-subscription-card
[owner-id dashboard dashcard card-or-id parameters]
(try
@@ -57,7 +74,7 @@
:dashcard-id (u/the-id dashcard)
:context :pulse ; TODO - we should support for `:dashboard-subscription` and use that to differentiate the two
:export-format :api
- :parameters (merge-default-values parameters)
+ :parameters parameters
:middleware {:process-viz-settings? true
:js-int-to-string? false}
:run (fn [query info]
@@ -83,12 +100,15 @@
[{pulse-creator-id :creator_id, :as pulse} dashboard & {:as _options}]
(let [dashboard-id (u/the-id dashboard)
dashcards (db/select DashboardCard :dashboard_id dashboard-id)
- ordered-dashcards (sort dashcard-comparator dashcards)]
+ ordered-dashcards (sort dashcard-comparator dashcards)
+ parameters (merge-default-values (params/parameters pulse dashboard))]
(for [dashcard ordered-dashcards]
(if-let [card-id (:card_id dashcard)]
- (execute-dashboard-subscription-card pulse-creator-id dashboard dashcard card-id (params/parameters pulse dashboard))
- ;; For virtual cards, return the viz settings map directly
- (-> dashcard :visualization_settings)))))
+ (execute-dashboard-subscription-card pulse-creator-id dashboard dashcard card-id parameters)
+ ;; For virtual cards, return just the viz settings map, with any parameter values substituted appropriately
+ (-> dashcard
+ (process-virtual-dashcard parameters)
+ :visualization_settings)))))
(defn- database-id [card]
(or (:database_id card)
diff --git a/src/metabase/pulse/parameters.clj b/src/metabase/pulse/parameters.clj
index cea37f678f8..94027f515cc 100644
--- a/src/metabase/pulse/parameters.clj
+++ b/src/metabase/pulse/parameters.clj
@@ -1,7 +1,9 @@
(ns metabase.pulse.parameters
"Utilities for processing parameters for inclusion in dashboard subscriptions."
(:require [clojure.string :as str]
+ [metabase.public-settings :as public-settings]
[metabase.public-settings.premium-features :refer [defenterprise]]
+ [metabase.shared.parameters.parameters :as shared.params]
[metabase.util :as u]
[metabase.util.urls :as urls]
[ring.util.codec :as codec]))
@@ -21,10 +23,13 @@
(the-parameters subscription dashboard)))
(defn value-string
- "Returns the value of a dashboard filter as a comma-separated string"
+ "Returns the value(s) of a dashboard filter, formatted appropriately."
[parameter]
- (let [values (u/one-or-many (or (:value parameter) (:default parameter)))]
- (str/join ", " values)))
+ (let [tyype (:type parameter)
+ values (or (:value parameter) (:default parameter))]
+ (try (shared.params/formatted-value tyype values (public-settings/site-locale))
+ (catch Throwable _
+ (shared.params/formatted-list (u/one-or-many values))))))
(defn dashboard-url
"Given a dashboard's ID and parameters, returns a URL for the dashboard with filters included"
diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj
index 59d33ce1e01..9958ee23936 100644
--- a/test/metabase/dashboard_subscription_test.clj
+++ b/test/metabase/dashboard_subscription_test.clj
@@ -304,10 +304,10 @@
(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&quarter_and_year=Q1-2021\\\"" true}
+ "<a class=\\\"title\\\" href=\\\"https://metabase.com/testmb/dashboard/\\d+\\?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021\\\"" true}
png-attachment]})
(mt/summarize-multipart-email #"Aviary KPIs"
- #"<a class=\"title\" href=\"https://metabase.com/testmb/dashboard/\d+\?state=CA&state=NY&quarter_and_year=Q1-2021\"")))))
+ #"<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]]
@@ -317,8 +317,8 @@
:attachments
[{:blocks [{:type "header", :text {:type "plain_text", :text "Aviary KPIs", :emoji true}}
{:type "section",
- :fields [{:type "mrkdwn", :text "*State*\nCA, NY"}
- {:type "mrkdwn", :text "*Quarter and Year*\nQ1-2021"}]}
+ :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 card-name
:rendered-info {:attachments false, :content true, :render/text true},
@@ -331,7 +331,7 @@
:elements [{:type "mrkdwn"
:text (str "<https://metabase.com/testmb/dashboard/"
dashboard-id
- "?state=CA&state=NY&quarter_and_year=Q1-2021|*Sent from Metabase Test*>")}]}]}]}
+ "?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021|*Sent from Metabase Test*>")}]}]}]}
(thunk->boolean pulse-results)))))}}))
(deftest mrkdwn-length-limit-test
@@ -424,3 +424,18 @@
(is (= [[1 "Rustic Paper Wallet" "Gizmo"]
[10 "Mediocre Wooden Table" "Gizmo"]]
(mt/rows results))))))))))
+
+(deftest substitute-parameters-in-virtual-cards
+ (testing "Parameters in virtual (text) cards should have parameter values substituted appropriately"
+ (mt/with-temp* [Dashboard [{dashboard-id :id :as dashboard} {:name "Params in Text Card Test"
+ :parameters [{:name "Category"
+ :slug "category"
+ :id "TEST_ID"
+ :type "category"
+ :default ["Doohickey" "Gizmo"]}]}]
+ DashboardCard [_ {:parameter_mappings [{:parameter_id "TEST_ID"
+ :target [:text-tag "foo"]}]
+ :dashboard_id dashboard-id
+ :visualization_settings {:text "{{foo}}"}}]]
+ (is (= [{:text "Doohickey and Gizmo"}]
+ (@#'metabase.pulse/execute-dashboard {:creator_id (mt/user->id :rasta)} dashboard))))))
diff --git a/test/metabase/pulse/parameters_test.clj b/test/metabase/pulse/parameters_test.clj
index 5ad67b99c84..abaf35cb133 100644
--- a/test/metabase/pulse/parameters_test.clj
+++ b/test/metabase/pulse/parameters_test.clj
@@ -1,22 +1,22 @@
(ns metabase.pulse.parameters-test
(:require [clojure.test :refer :all]
[metabase.pulse.parameters :as params]
- [metabase.pulse.test-util :refer :all]
+ [metabase.pulse.test-util :refer [test-dashboard]]
[metabase.test :as mt]))
(deftest value-string-test
(testing "If a filter has multiple values, they are concatenated into a comma-separated string"
- (is (= "CA, NY"
+ (is (= "CA, NY and NJ"
(params/value-string (-> test-dashboard :parameters first)))))
- (testing "If a filter has a single default value, it is returned unmodified"
- (is (= "Q1-2021"
+ (testing "If a filter has a single default value, it is formatted appropriately"
+ (is (= "Q1, 2021"
(params/value-string (-> test-dashboard :parameters second))))))
(deftest dashboard-url-test
(mt/with-temporary-setting-values [site-url "https://metabase.com"]
(testing "A valid dashboard URL can be generated with filters included"
- (is (= "https://metabase.com/dashboard/1?state=CA&state=NY&quarter_and_year=Q1-2021"
+ (is (= "https://metabase.com/dashboard/1?state=CA&state=NY&state=NJ&quarter_and_year=Q1-2021"
(params/dashboard-url 1 (:parameters test-dashboard)))))
(testing "If no filters are set, the base dashboard url is returned"
diff --git a/test/metabase/pulse/test_util.clj b/test/metabase/pulse/test_util.clj
index 18d23123c48..0893a3f3f87 100644
--- a/test/metabase/pulse/test_util.clj
+++ b/test/metabase/pulse/test_util.clj
@@ -145,7 +145,7 @@
[{:name "State",
:slug "state",
:id "63e719d0",
- :default ["CA", "NY"],
+ :default ["CA", "NY", "NJ"],
:type "string/=",
:sectionId "location"}
{:name "Quarter and Year",
--
GitLab