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

@@ -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;

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()),
       );
     }
 

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");
+    });
+  });
 });

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))))))

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"))))

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)

src/metabase/pulse/parameters.clj

 (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"

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&amp;state=NY&amp;quarter_and_year=Q1-2021\\\"" true}
+                                              "<a class=\\\"title\\\" href=\\\"https://metabase.com/testmb/dashboard/\\d+\\?state=CA&amp;state=NY&amp;state=NJ&amp;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&amp;state=NY&amp;quarter_and_year=Q1-2021\"")))))
+                                                #"<a class=\"title\" href=\"https://metabase.com/testmb/dashboard/\d+\?state=CA&amp;state=NY&amp;state=NJ&amp;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))))))

test/metabase/pulse/parameters_test.clj

 (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"

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",